POP QUIZ: What’s wrong with this code – part 2

typedef bool (FUNC)(void *);

class Foo
{
    virtual FUNC Method;

    void *m_p;
};

bool Foo::Method(void *p)
{
    return  p == m_p;
};

Comments

# Henk de Koning said on May 15, 2008 6:45 AM:

A bunch of things come to mind:

1) there's no implementation of Method returning FUNC, (supposing it is an (wrong) attempt to declare a method);

2) everything is implicitly private so virtual doesn't make sense;

3) if the intention is to have an implementation that is virtual, it is not good style to touch private variables. You will probably want to make m_p protected in that case;

4) m_p is never initialized and will have a random value in a release build (or worse, it'll disclose a previous value of whatever used to be in memory);

5) Method is never initialized (see 4). This one is worse as invoking it would result in arbitrary code being executed;

-- Henkk

# leppie said on May 15, 2008 8:13 AM:

The declaration specifies a function pointer, and your implementation provides a function.

# ASP.NET Debugging : POP QUIZ: What???s wrong with this code ??? part 2 said on May 15, 2008 8:44 AM:

PingBack from http://blogs.msdn.com/tom/archive/2008/05/15/pop-quiz-what-s-wrong-with-this-code-part-2.aspx

# Ahem said on May 15, 2008 10:10 AM:

typedef bool (FUNC)(void *);

should be

typedef bool (*FUNC)(void *);

no? You're using typedef to define a pointer for a function, and not the function itself.

# mooglegiant said on May 15, 2008 12:38 PM:

Well, im not too sure what are you getting at.  I am assuming you are trying to check if the address of the two pointers is the same.  But I'm pretty sure that won't work on void pointers and you will have to type cast them first.

# HeartattacK said on May 15, 2008 12:59 PM:

1. Method is virtual, yet access is private. A child can not use the default implemention. Furthermore, as m_p is also private, the child can't provide  an implementation that uses m_p.

2. m_p has not been set anywhere, yet it is being compared with.

# Anonymous said on May 15, 2008 4:52 PM:

The only mistake I found in a glance was in the last statement return p == m_p; No comparison operator can be used in this manner esp in a return statement

Correct me if I am wrong

Leave a Comment

Name (required) 

Your URL (optional)

Comments (required) 

  

Enter Code Here: Required

Remember Me?

Search

This Blog

• Home
• Email