Thread: using the C++ delete operator

The above code doesn't use the new operator (duh, I know, but you never know). Re-check that you have allocated memory (maybe the new threw an exception but you didn't exit the program?

Maybe you call it twice? Try this

Code:

void FreeMemory(void *mem)
{
     if (mem == NULL) return;
     delete mem;
     mem = NULL;
}

this ensures you cannot call it twice.

Just don't use uninitialized variables. The C++ language doesn't protect you in any way if you do.

The compiler can even normally tell if you are using uninitialized variables if you enable warnings:

Code:

void FreeMemory(void *mem)
{
    delete mem;
}

int main()
{
    char *p;
    FreeMemory(p);
}

Comeau C/C++ 4.3.10.1 (Oct 6 2008 11:28:09) for ONLINE_EVALUATION_BETA2
Copyright 1988-2008 Comeau Computing. All rights reserved.
MODE:strict errors C++ C++0x_extensions

"ComeauTest.c", line 9: warning: variable "p" is used before its value is set
FreeMemory(p);

Another issue could be that it might be incorrect to pass a void* to delete.

As to assigning NULL to mem, this is pointless since the copy of the pointer goes out of scope anyway. Checking for NULL before deleting is also pointless, and generally there is no way to check if the pointer, should it be non-NULL, points to something deletable. Generally one would use containers, smart pointers etc, so that delete wouldn't be used anywhere in the code in the first place.

Actually I am surprised it compiled. Most of the time if you are going to delete a void *, you have to cast it to the type that you are actually deleting. IOW:

Code:

MyClass *ptr = new MyClass;
FreeMemory((void*)ptr);


...
void FreeMemory(void *pMem)
{
      delete pMem; // this is no good
      delete (MyClass *)pMem; // this should work
}

Agreed. This FreeMemory function is an abomination. Remove this function and just call delete at the appropriate place. Or alternatively, make it templated on the type of item being deleted.
No C++ program should ever have a function that performs a delete without knowing the type of the thing it is deleting.

In addition to that, fix the test case. You're probably trying to delete the address of some local variable or something like that.

Oh and C_ntua, consider yourself shot for those two redundancies you suggested.

Another problem with managing things yourself is that there isn't really a practical way to differentiate between arrays (which require a call to delete []) and single objects.

Conclusion: Use RAII. Always.

Originally Posted by EVOEx

Lol. I am fairly ashamed to admit I missed that mem was a parameter. And even a non-reference one...

Yeah I hope he realises I was being partially humourous.
Probably anyone with more than a few hundred posts on these forums should at least know by now that there's no need to check for NULL first. I'm sure C_ntua is normally is a bit more clued up than that. Damn those brain worms eh!...

No, since the check is for a null pointer, not "an uninitialised pointer", and there is still the issue of trying to use delete on a pointer to void.