Welcome to MSDN Blogs Sign in | Join | Help
C++ Still Bites Back

I don’t know how many times I’ve thought to myself that I knew it all.  Many times I’ve coded something that I was absolutely sure was correct only to find out I was drastically wrong. 

What’s wrong with this code?

class Object

{

public:

      Object()

            : m_pdata( new BYTE[100] )

      {

      }

      ~Object()

      {

            delete[] m_pdata;

      }

 

private:

      BYTE *m_pdata;

};

 

class Interface

{

public:

      static Object* AllocateObject() { return new Object(); }

      static void DeleteObject( void* pObject )

         { delete pObject; }

};

 

...

 

void Function()

{

      Object* pObject = Interface::AllocateObject();

     

      // do something

 

      Interface::DeleteObject( pObject );

}

 

Answer

It will leak memory. DeleteObject takes in a void* which when deleted will not call the destructor for the Object class and m_pdata leaks.

I made this mistake when I added a new type of object that needed to be allocated from an interface. Then I tried to consolidate the “Delete” functions into a single one (read: "make better"). I didn’t see it when I coded because the classes I tested it with had trivial destructors and freeing the memory was enough. It wasn’t until testers tried more complicated scenarios did the bug manifest itself (read: "made totally worse").

Posted: Friday, May 30, 2008 1:14 AM by Chris Becker
Filed under:

Comments

Tom said:

Interesting bug... I'm having a hard time in general understanding the combination of delete with a void*.  Is there even any case where this is a valid thing to do?  Too bad you didn't get a compiler warning on that statement.

# May 29, 2008 8:30 PM

Chris Becker said:

When you overload the "delete" operator for a class, it takes a "void*" which is why I thought this would work. (Ex: http://www.devx.com/tips/Tip/5608)

When you use the delete keyword, the compiler checks for the existance of a destructor and calls that first. There is a warning if you have prototyped but not defined a class and then try and delete its pointer (C4150). But conversion to a void* is valid and deleting a void* is valid, so it's not going to see that you've made a mistake.

When you call delete on a "void*" it's really the same thing as freeing the memory. I suppose this could have its uses, but it's ugly.

# May 29, 2008 8:48 PM

Tom said:

My read of the C++ Standard (Section 5.3.5.2) is that the behavior of delete on a void* is undefined:

"...the value of the operand of delete shall be a pointer to a nonarray object or a pointer to a subobject (1.8) representing a base class of such an object (clause 10). If not, the behavior is undefined."

I would interpret the void* to fall into the latter category.  What do you think?

# May 29, 2008 10:08 PM

Chris Becker said:

I'd agree, the lesson is don't delete void pointers.

# May 29, 2008 10:11 PM
Leave a Comment

(required) 

(required) 

(optional)

(required) 

  
Enter Code Here: Required

Comment Notification

If you would like to receive an email when updates are made to this post, please register here

Subscribe to this post's comments using RSS

Page view tracker