Printable View
The code above gives following error under MSVS .Net 2003:Code:#include <iostream>
using namespace std;
template <typename M>
void my_delete(M *&p)
{
if (p) delete p;
p = 0;
}
int main()
{
int* ptr;
cout << hex << ptr << endl;
ptr = new int;
cout << hex << ptr << endl;
my_delete(ptr);
cout << hex << ptr << endl;
return 0;
}
test4.cpp(17) : error C2664: 'my_delete' : cannot convert parameter 1 from 'int *' to 'void *& '
A reference that is not to 'const' cannot be bound to a non-lvalue
Regardless of why or why it does not work, this is very unsafe. Remake your delete function using templates. Calling delete on a void pointer is undefined.
Thanks Elysia!
It's working fine and much better now :)
Also, you shouldn't need to check the value of p before deleting it. Calling delete on a NULL pointer should have no effect.
Though you can optimize it slightly by only calling delete and setting the pointer to NULL if p != NULL! Not that it really matters, however.
Thanks Elysia! Thanks cpjust!
>> Though you can optimize it slightly by only calling delete and setting the pointer to NULL if p != NULL!
Wouldn't a proper implementation of delete already do that for you?
The "optimization" if you can call it that saves time by calling delete in the first place and setting the pointer to NULL.
At least in some implementations, the C runtime and the C++ one is quite big with lots of code to do a single thing. Just look at new/delete with Microsoft's implementation.
Think of how many thousands of instructions you'll save from not calling it at all (not to mention you save a function call).
No, it's not much of an optimization at all since it will be lightning fast anyway.
You might also consider how often your code will attempt to delete a NULL pointer? If you think it will be a highly unlikely occurance, then removing the if statement will save a few clock ticks/call.
More likely, it will save time since it will save the time it takes to call and execute delete plus save 0 to the pointer.
The more often it's called, the more it would save. Theoretically anyway.
The check for null inside the if has a cost. If the pointer is rarely null, then that cost will outweigh any potential benefit from skipping the code inside the block.
In addition, my question was whether the delete implementation checks for null itself. If it does (I don't know the implementation, it was an honest question), then that duplicate check means that only the assignment in the deleter function is saved. So again, the pointer would have to be null often enough that the cost of the extra comparison was less than the benefit of the skipped assignment.
I agree with you that it probably doesn't matter, but I would prefer to do it without the check for null.
The delete operator is required by the standard to have no effect if invoked on a NULL pointer. That would imply the implementation of that operator requires a check.Quote:
Originally Posted by Daved
In practice, some very old C implementations (significantly predating the 1989/90 C standard) would fall over as a side effect of free((void *)NULL) and some early C++ implementations (predating the 1998 standard by at least a decade) would fall over with delete NULL because they were implemented using those older C compilers/libraries. You will probably never use those implementations unless you beg/borrow vendor-specific compilers from museums.
I just wanted a delete that would set the pointer to NULL after freeing the memory.
Standard delete does not do this:
Code:// MSVS .Net 2003
#include <iostream>
using namespace std;
int main()
{
int* p;
cout << "Grabage: " << p << endl;
p = new int(0);
cout << "After new: " << p << endl;
delete p;
cout << "After delete: " << p << endl;
return 0;
}
How often do you have pointers that require this?
BTW, here is Stroustrup's comments on the subject, including his implementation of such a function:
http://www.research.att.com/~bs/bs_f...ml#delete-zero
No it's not that, if this is very often or not?
I just wanted the pointers, that don't point to valid memory, to become NULL.
And the function I made up should be good enough for me. Although after the comments that
I received here I think I better remove that if statement!
I don't believe you have to do anything at all. If the if statement is better or not, we can't really say. The best way is to use a profiler, but that would be a little overkill, wouldn't it?
Unless you're deleting millions of pointers/second, 1 more or less instruction will be completely unnoticable.
>> If the if statement is better or not, we can't really say.
We can discuss it and come to the best logical conclusion possible. My logic tells me it is better to leave it out. Apparently Stroustrup agrees, so I would take it out. Definitely not a big deal but why not make the best decision possible?
If you don't care about the optimization, then use the clearer version, which again is the one without the if, IMO.
Why not just use a macro like
Code:#define SAFE_DELETE(p) { delete p; p = NULL; }
Because macros do not respect scope, are not type-safe, can be used on a void pointer, etc etc.
Macros like yours also will also result in a compilation error with code like;
although that's a property of your macro, which can be addressed by;Code:if (some_condition)
SAFE_DELETE(some_pointer);
else
something_else();
Code:#define SAFE_DELETE(p) do {delete p; p = NULL;} while (0)
I don't like the name SAFE_DELETE. An individual call to SAFE_DELETE is no safer than just calling delete. After all, that's the first thing SAFE_DELETE does anyway.
I would take a lesson from Delphi which has FreeAndNil, calling this DELETE_AND_NULL, to properly say what it does.
Then I would proclaim that it is so ugly and pointless that I would throw it away and just use delete instead. I would only NULL the pointer when I actually need to, which is almost never if you properly use RAII.