Is it good practice to NULL a pointer after deleting it?
Asked Answered
F

18

185

I'll start out by saying, use smart pointers and you'll never have to worry about this.

What are the problems with the following code?

Foo * p = new Foo;
// (use p)
delete p;
p = NULL;

This was sparked by an answer and comments to another question. One comment from Neil Butterworth generated a few upvotes:

Setting pointers to NULL following delete is not universal good practice in C++. There are times when it is a good thing to do, and times when it is pointless and can hide errors.

There are plenty of circumstances where it wouldn't help. But in my experience, it can't hurt. Somebody enlighten me.

Fremitus answered 18/12, 2009 at 22:48 Comment(14)
I am curious too, you would think not setting a deleted pointer to NULL is usually a cause of hidden errors.Stepfather
@Mark Ransom: Just for curiosity (I'm a C# programmer) what happens when you try to use a pointer after it's deleted in C++? An access violation?Kela
@Andre: Technically, it's undefined. What's likely to happen is that you access the same memory as before, but it may now be used by something else. If you delete memory twice, it's likely to screw up your program execution in a hard-to-find way. It is safe to delete a null pointer, though, which is one reason zeroing a pointer can be good.Evania
@André Pena, it's undefined. Often it's not even repeatable. You set the pointer to NULL to make the error more visible when debugging, and maybe to make it more repeatable.Fremitus
@André: No one knows. It is Undefined Behavior. It might crash with an access violation, or it might overwrite memory used by the rest of the application. The language standard makes no guarantees of what happens, and so you cannot trust your application once it has happened. It could have fired the nuclear missiles or formatted your harddrive. it can corrupt your app's memory, or it might make demons fly out of your nose. All bets are off.Assibilate
The flying demons are a feature, not a bug.Shugart
Thanks @David, @Mark and @Jalf. I think I got it: As I deleted a pointer, I'm saying to the operation system I'm releasing that memory block so another application (or my own) may have using that memory just after I deleted, leading to a (weird for me :]) "unpredictable" outcome.Kela
The best in my opinion is to set to 0, and insert assert(foo != 0); if the logic in your program forbids double deletion, instead of relying on the runtime system's undocumented behavior.Slush
Duplicate: #1880050Chloramphenicol
Come'on, Mark. You should have guessed that this had been asked before.Fibroin
@Steve Jessop, thanks for the link - my question does seem to be a duplicate. I wonder why this question seems to be getting so much more discussion than the previous one? This answer seems to answer my question, by pointing out that it obscures double delete bugs - #1880050Fremitus
I think the answers you get depend a lot on who happens to be online when you ask. Questions disappear into the depths fairly quickly on Stackoverflow, and the search function doesn't stop the same issues coming around repeatedly. The meaning of "inline" in C++ comes up a lot, for example, as does "why is i = ++i undefined?". I don't think it's a big problem, duplicate questions can always be merged later.Chloramphenicol
This question is not a duplicate because the other question is about C and this one is about C++. A lot of the answers hinge on things like smart pointers, which aren't available in C++.Allurement
Stroustrup talks a bit about this: Why doesn't delete zero out its operand? In which he says "C++ explicitly allows an implementation of delete to zero out an lvalue operand, and I had hoped that implementations would do that, but that idea doesn't seem to have become popular with implementers. If you consider zeroing out pointers important, consider using a destroy function: template<class T> inline void destroy(T*& p) { delete p; p = 0; }"Penstemon
P
122

Setting a pointer to 0 (which is "null" in standard C++, the NULL define from C is somewhat different) avoids crashes on double deletes.

Consider the following:

Foo* foo = 0; // Sets the pointer to 0 (C++ NULL)
delete foo; // Won't do anything

Whereas:

Foo* foo = new Foo();
delete foo; // Deletes the object
delete foo; // Undefined behavior 

In other words, if you don't set deleted pointers to 0, you will get into trouble if you're doing double deletes. An argument against setting pointers to 0 after delete would be that doing so just masks double delete bugs and leaves them unhandled.

It's best to not have double delete bugs, obviously, but depending on ownership semantics and object lifecycles, this can be hard to achieve in practice. I prefer a masked double delete bug over UB.

Finally, a sidenote regarding managing object allocation, I suggest you take a look at std::unique_ptr for strict/singular ownership, std::shared_ptr for shared ownership, or another smart pointer implementation, depending on your needs.

Piezochemistry answered 18/12, 2009 at 23:1 Comment(6)
Your application won't always crash on a double delete. Depending on what happens between the two deletes, anything could happen. Most likely, you'll corrupt your heap, and you'll crash at some point later in a completely unrelated piece of code. While a segfault is usually better than silently ignoring the error, the segfault isn't guaranteed in this case, and it's of questionable utility.Offwhite
The problem here is the fact that you have a double delete. Making the pointer NULL just hides that fact it does not correct it or make it any safer. Imagaine a mainainer comming back a year later and seeing foo deleted. He now believes he can re-use the pointer unfortunately he may miss the second delete (it may not even be in the same function) and now the re-use of the pointer now gets trashed by the second delete. Any access after the second delete is now a major problem.Asthenosphere
It's true the setting the pointer to NULL can mask a double delete bug. (Some might consider this mask to actually be a solution--it is, but not a very good one since it doesn't get to the root of the problem.) But not setting it to NULL masks the far (FAR!) more common problems of accessing the data after it has been deleted.Allurement
AFAIK, std::auto_ptr has been deprecated in the upcoming c++ standardPyrethrin
I wouldn't say deprecated, it makes it sound like the idea is just gone. Rather, it's being replaced with unique_ptr, which does what auto_ptr tried to do, with move semantics.Paquette
good point, "being replaced with" is the right thing to say ;-)Pyrethrin
A
64

Setting pointers to NULL after you've deleted what it pointed to certainly can't hurt, but it's often a bit of a band-aid over a more fundamental problem: Why are you using a pointer in the first place? I can see two typical reasons:

  • You simply wanted something allocated on the heap. In which case wrapping it in a RAII object would have been much safer and cleaner. End the RAII object's scope when you no longer need the object. That's how std::vector works, and it solves the problem of accidentally leaving pointers to deallocated memory around. There are no pointers.
  • Or perhaps you wanted some complex shared ownership semantics. The pointer returned from new might not be the same as the one that delete is called on. Multiple objects may have used the object simultaneously in the meantime. In that case, a shared pointer or something similar would have been preferable.

My rule of thumb is that if you leave pointers around in user code, you're Doing It Wrong. The pointer shouldn't be there to point to garbage in the first place. Why isn't there an object taking responsibility for ensuring its validity? Why doesn't its scope end when the pointed-to object does?

Assibilate answered 18/12, 2009 at 22:55 Comment(14)
So you're making the argument that there shouldn't have been a raw pointer in the first place, and anything involving said pointer shouldn't be blessed with the term "good practice"? Fair enough.Fremitus
Well, more or less. I wouldn't say that nothing involving a raw pointer can be termed a good practice. Just that it's the exception rather than the rule. Usually, the presence of the pointer is an indicator that there's something wrong at a deeper level.Assibilate
but to answer the immediate question, no, I don't see how setting pointers to null can ever cause errors.Assibilate
Your first paragraph after the bulleted section forced me to upvote. Most of your pointer problems will disappear whether or not you null them if you toasterize your functionality.Enesco
I disagree -- there are cases when a pointer is good to use. For example, there are 2 variables on the stack and you want to choose one of them. Or you want to pass an optional variable to a function. I would say, you should never use a raw pointer in conjunction with new.Chavira
True, but Boost isn't always an option, and sometimes the simplicity of a pointer may be preferable. (Everyone knows that a pointer may point to an object or null. People may not be familiar with boost::optional)Assibilate
I completely disagree that setting them to NULL is a Band-Aid. It's actually a valid method of catching bugs. Using an RAII wrapper is certainly best whenever possible, but sometimes you have to implement your own wrapper, in which case, this is a valid question.Allurement
What do you mean by "your own wrapper"? Why shouldn't you use RAII for that wrapper? It is a band-aid in that it fixes errors that only occur because you didn't fix the underlying problem (that the variable is still in scope)Assibilate
@jalf: Sometimes there isn't a pre-made wrapper class for the type of resource you need, in which case you have to write your own. The implementation of that will require a pointer (or other resource "handle").Allurement
@Adrian: Yes, but the implementation won't expose that pointer outside the scope in which it lives. If the pointer is a private member of the RAII wrapper you're writing, it will go out of scope the moment the wrapper does, and so there is nothing to set to NULL.Assibilate
@jalf: So? The implementation might still need to deal with that pointer, so should it set the pointer to NULL? I say yes.Allurement
when a pointer has gone out of scope, I don't see how anything or anyone might need to deal with it.Assibilate
"I don't see how setting pointers to null can ever cause errors" -- if it does then the code was arguably broken already, because the delete operator on a non-const lvalue operand is permitted to modify the value and in particular is permitted to set it to a null pointer. So if setting it to null causes an error, then the original code was pointlessly non-portable. Not that any implementation actually does clear pointers on delete afaik, hence only "arguably" broken.Chloramphenicol
@Chavira If there are two local variables and you want to choose one, the correct tool is a reference, not a pointer. Aside from the added safety, there's also a chance that the variable might then not have to be put on the stack in the first place and could possibly be kept in a register instead. Taking the address of something forces the compiler to put that something in memory when it might otherwise have not needed to.Spandrel
B
47

I've got an even better best practice: Where possible, end the variable's scope!

{
    Foo* pFoo = new Foo;
    // use pFoo
    delete pFoo;
}
Bonanza answered 18/12, 2009 at 22:49 Comment(11)
Yes, RAII is your friend. Wrap it in a class and it becomes even simpler. Or don't handle memory yourself at all by using the STL!Euphroe
Yes indeed, that's the best option. Doesn't answer the question though.Fremitus
This seems to be just to be a by-product of using function scopes period, and doesn't really address the problem. When you're using pointers, you're usually passing copies of them several layers deep and then your method is really meaningless in an effort to address the problem. While I agree that good design will help you isolate the errors, I don't think that your method is the primary means to that end.Enesco
Come to think of it, if you could do this, why wouldn't you just forget the heap and pull all of your memory off the stack?Enesco
@San Jacinto: That's a scope created explicitly to manage the pointer's lifetime, it's unrelated to function scope.Bonanza
I wouldn't say it's unrelated. They follow the same rules. What I'm saying is that you don't gain much except making your code sloppier because you're usually going to be using a pointer deeper in the call stack than this particular scope. In my experience, when I look at someone's pointer problem, it usually isn't even in the same function where they allocated the memory. If it is in the same function, it's usually because the function is TOOOOO LOOOONNG, and your method only encourages this. I find it interesting, but I can't upvote. I don't see the utility.Enesco
Code evolves. Someone will eventually add code in that scope but after the delete. (The new code may need to be inside the block rather than after it to get access to other variables that live only at that scope.) In that case, it'll be better if you set pFoo to nullptr (or NULL).Allurement
@Don, if the use of the pointer variable is confined to a scope, then just use stack allocated variable.Hobnob
My example is intentionally minimal. For example, instead of new, maybe the object is created by a factory , in which case it can't go on the stack. Or maybe it's not created at the beginning of the scope, but located in some structure. What I'm illustrating is that this approach will find any misuse of the pointer at compile time, whereas NULLing it will find any misuse at run time.Bonanza
This is awful code. If an exception is thrown while you're using foo, it will NEVER get freed.Savannasavannah
@Savannasavannah did you even read the question? Yes, RIAA is a better pattern, no, that's not what this question is about.Bonanza
A
37

I always set a pointer to NULL (now nullptr) after deleting the object(s) it points to.

  1. It can help catch many references to freed memory (assuming your platform faults on a deref of a null pointer).

  2. It won't catch all references to free'd memory if, for example, you have copies of the pointer lying around. But some is better than none.

  3. It will mask a double-delete, but I find those are far less common than accesses to already freed memory.

  4. In many cases the compiler is going to optimize it away. So the argument that it's unnecessary doesn't persuade me.

  5. If you're already using RAII, then there aren't many deletes in your code to begin with, so the argument that the extra assignment causes clutter doesn't persuade me.

  6. It's often convenient, when debugging, to see the null value rather than a stale pointer.

  7. If this still bothers you, use a smart pointer or a reference instead.

I also set other types of resource handles to the no-resource value when the resource is free'd (which is typically only in the destructor of an RAII wrapper written to encapsulate the resource).

I worked on a large (9 million statements) commercial product (primarily in C). At one point, we used macro magic to null out the pointer when memory was freed. This immediately exposed lots of lurking bugs that were promptly fixed. As far as I can remember, we never had a double-free bug.

Update: Microsoft believes that it's a good practice for security and recommends the practice in their SDL policies. Apparently MSVC++11 will stomp the deleted pointer automatically (in many circumstances) if you compile with the /SDL option.

Allurement answered 19/12, 2009 at 0:24 Comment(0)
I
12

Firstly, there are a lot of existing questions on this and closely related topics, for example Why doesn't delete set the pointer to NULL?.

In your code, the issue what goes on in (use p). For example, if somewhere you have code like this:

Foo * p2 = p;

then setting p to NULL accomplishes very little, as you still have the pointer p2 to worry about.

This is not to say that setting a pointer to NULL is always pointless. For example, if p were a member variable pointing to a resource who's lifetime was not exactly the same as the class containing p, then setting p to NULL could be a useful way of indicating the presence or absence of the resource.

Infundibuliform answered 18/12, 2009 at 22:56 Comment(10)
I agree that there are times when it won't help, but you seemed to imply that it could be actively harmful. Was that your intent, or did I read it wrong?Fremitus
Whether there s a copy of the pointer is irrelevant to the question whether the pointer variable should be set to NULL. Setting it to NULL is a good practice on the same grounds cleaning the dishes after you're done with dinner is a good practice - while it's not a safeguard against all bugs a code can have, it does promote good code health.Johst
@Franci Lots of people seem to disagree with you. And whether there is a copy certainly is relevant if you try to use the copy after you deleted the original.Infundibuliform
Franci, there's a difference. You clean dishes because you use them again. You don't need the pointer after you delete it. It should be the last thing you do. Better practice is to avoid the situation altogether.Paquette
@Gman so I can't reuse a pointer variable? I allocate memory to that variable once and I never touch it after I delete that value?Johst
The assignment example is different that talking about whether the pointer should be NULLed after it's deleted. Whether you set p to NULL depends on whether you're transferring ownership. This is orthogonal to the question at hand.Allurement
You can re-use a variable, but then it's no longer a case of defensive programming; it's how you designed the solution to the problem at hand. The OP is discussing whether this defensive style is something we should strive for, not of we'll ever set a pointer to null. And ideally, to your question, yes! Don't use pointers after you delete them!Paquette
@gma by that logig a pointer variable should be associated with the first value it is assigned and never be used for other values since it violates the principles of the defensive programming. But a pointer variable is no different than an int variables - it's just a memory storage for a value. if you want to be consistent you should not reuse other variables either and have them associated with the first value assigned to them. why are you feeling ok to set a counter to zero after you exit a loop, but you don't apply same to pointers?Johst
+1 for 'For example, if p were a member variable pointing to a resource who's lifetime was not exactly the same as the class containing p, then setting p to NULL could be a useful way of indicating the presence or absence of the resource.' which actually demonstrates an interesting point and accepts that pointers can be useful.Borroff
@Neil if people always agreed with each other, there would never be a discussion, right? :-) but back to the topic - take the following code: char a[10]; char b[5]; int i = 7; a[i] = 'x'; int j = i; i = 0; b[j] = 'z';. Should we not set i to 0, because there's a copy of its value and using that copy can lead to a bug? how are i and j here different from a pointer? the semantic of their values is a memory address; the only difference is it's a relative memory address, not an absolute memory address. but that does not make it necessarily a valid memory address...Johst
G
8

If there is more code after the delete, Yes. When the pointer is deleted in a constructor or at the end of method or function, No.

The point of this parable is to remind the programmer, during run-time, that the object has already been deleted.

An even better practice is to use Smart Pointers (shared or scoped) which automagically delete their target objects.

Grayish answered 18/12, 2009 at 22:52 Comment(1)
Everyone (including the original questioner) agrees that smart pointers are the way to go. Code evolves. There might not be more code after the delete when you first right it, but that's likely to change over time. Putting in the assignment helps when that happens (and costs almost nothing in the mean time).Allurement
I
3

As others have said, delete ptr; ptr = 0; is not going to cause demons to fly out of your nose. However, it does encourage the usage of ptr as a flag of sorts. The code becomes littered with delete and setting the pointer to NULL. The next step is to scatter if (arg == NULL) return; through your code to protect against the accidental usage of a NULL pointer. The problem occurs once the checks against NULL become your primary means of checking for the state of an object or program.

I'm sure that there is a code smell about using a pointer as a flag somewhere but I haven't found one.

Indigestive answered 18/12, 2009 at 23:21 Comment(1)
There's nothing wrong with using a pointer as a flag. If you're using a pointer, and NULL isn't a valid value, then you probably should be using a reference instead.Allurement
B
2

"There are times when it is a good thing to do, and times when it is pointless and can hide errors"

I can see two problems: That simple code:

delete myObj;
myobj = 0

becomes to a for-liner in multithreaded environment:

lock(myObjMutex); 
delete myObj;
myobj = 0
unlock(myObjMutex);

The "best practice" of Don Neufeld don't apply always. E.g. in one automotive project we had to set pointers to 0 even in destructors. I can imagine in safety-critical software such rules are not uncommon. It is easier (and wise) to follow them than trying to persuade the team/code-checker for each pointer use in code, that a line nulling this pointer is redundant.

Another danger is relying on this technique in exceptions-using code:

try{  
   delete myObj; //exception in destructor
   myObj=0
}
catch
{
   //myObj=0; <- possibly resource-leak
}

if (myObj)
  // use myObj <--undefined behaviour

In such code either you produce resource-leak and postpone the problem or the process crashes.

So, this two problems going spontaneously through my head (Herb Sutter would for sure tell more) make for me all the questions of the kind "How to avoid using smart-pointers and do the job safely with normal pointers" as obsolete.

Biracial answered 18/12, 2009 at 22:48 Comment(6)
I fail to see, how the 4-liner is significantly more complex than a 3-liner (one should use lock_guards anyway) and if your destructor throws you are in trouble anyway.Poulos
When I first saw this answer I didn't understand why you would want to null a pointer in a destructor, but now I do - it's for the case where the object owning the pointer gets used after it's deleted!Fremitus
@Mark Ransom You would also want to do it for security applications because maybe that pointer leads to the array of chars that stores your credit card info. Instead of having to search through the entire memory dump to find that string of credit card info, you now only have to find and dereference that pointer. Maybe I'm wrong and you'd still have to do a significant amount of sifting through the memory dump, but I feel like nonNULL dead pointers make a hacker's job easier.Affixation
@Affixation if you're that worried about security, you erase the data by overwriting it before you delete the object.Fremitus
@Mark Ransom What's the issue with overwriting after deleting the object? Ah. Me being stupid and not realizing the nullptr dereference. I still feel like getting rid of any usable data for a hacker to "feel their way around" (note common memory locations for specific functions) a memory dump is good, unless you're doing security through obscurity with massive amounts of trash data any time you might have a nullptr dereference.Affixation
@Affixation you can't overwrite the memory after the delete because it doesn't belong to you anymore. That's undefined behavior. If another object gets allocated to that memory before you start clearing it, very bad things will happen.Fremitus
J
2

I'll change your question slightly:

Would you use an uninitialized pointer? You know, one that you didn't set to NULL or allocate the memory it points to?

There are two scenarios where setting the pointer to NULL can be skipped:

  • the pointer variable goes out of scope immediately
  • you have overloaded the semantic of the pointer and are using its value not only as a memory pointer, but also as a key or raw value. this approach however suffers from other problems.

Meanwhile, arguing that setting the pointer to NULL might hide errors to me sounds like arguing that you shouldn't fix a bug because the fix might hide another bug. The only bugs that might show if the pointer is not set to NULL would be the ones that try to use the pointer. But setting it to NULL would actually cause exactly the same bug as would show if you use it with freed memory, wouldn't it?

Johst answered 18/12, 2009 at 23:4 Comment(1)
(A) "sounds like arguing that you shouldn't fix a bug" Not setting a pointer to NULL is not a bug. (B) "But setting it to NULL would actually cause exactly the same bug" No. Setting to NULL hides double delete. (C) Summary: Setting to NULL hides double delete, but exposes stale references. Not setting to NULL can hide stale references, but exposes double deletes. Both sides agree that the real problem is to fix stale-references and double-deletes.Inpour
P
2

If you have no other constraint that forces you to either set or not set the pointer to NULL after you delete it (one such constraint was mentioned by Neil Butterworth), then my personal preference is to leave it be.

For me, the question isn't "is this a good idea?" but "what behavior would I prevent or allow to succeed by doing this?" For example, if this allows other code to see that the pointer is no longer available, why is other code even attempting to look at freed pointers after they are freed? Usually, it's a bug.

It also does more work than necessary as well as hindering post-mortem debugging. The less you touch memory after you don't need it, the easier it is to figure out why something crashed. Many times I have relied on the fact that memory is in a similar state to when a particular bug occurred to diagnose and fix said bug.

Protease answered 18/12, 2009 at 23:26 Comment(0)
E
2

Explicitly nulling after delete strongly suggests to a reader that the pointer represents something which is conceptually optional. If I saw that being done, I'd start worrying that everywhere in the source the pointer gets used that it should be first tested against NULL.

If that's what you actually mean, it's better to make that explicit in the source using something like boost::optional

optional<Foo*> p (new Foo);
// (use p.get(), but must test p for truth first!...)
delete p.get();
p = optional<Foo*>();

But if you really wanted people to know the pointer has "gone bad", I'll pitch in 100% agreement with those who say the best thing to do is make it go out of scope. Then you're using the compiler to prevent the possibility of bad dereferences at runtime.

That's the baby in all the C++ bathwater, shouldn't throw it out. :)

Exciter answered 19/12, 2009 at 1:17 Comment(0)
M
2

In a well structured program with appropriate error checking, there is no reason not to assign it null. 0 stands alone as a universally recognized invalid value in this context. Fail hard and Fail soon.

Many of the arguments against assigning 0 suggest that it could hide a bug or complicate control flow. Fundamentally, that is either an upstream error (not your fault (sorry for the bad pun)) or another error on the programmer's behalf -- perhaps even an indication that program flow has grown too complex.

If the programmer wants to introduce the use of a pointer which may be null as a special value and write all the necessary dodging around that, that's a complication they have deliberately introduced. The better the quarantine, the sooner you find cases of misuse, and the less they are able to spread into other programs.

Well structured programs may be designed using C++ features to avoid these cases. You can use references, or you can just say "passing/using null or invalid arguments is an error" -- an approach which is equally applicable to containers, such as smart pointers. Increasing consistent and correct behavior forbids these bugs from getting far.

From there, you have only a very limited scope and context where a null pointer may exist (or is permitted).

The same may be applied to pointers which are not const. Following the value of a pointer is trivial because its scope is so small, and improper use is checked and well defined. If your toolset and engineers cannot follow the program following a quick read or there is inappropriate error checking or inconsistent/lenient program flow, you have other, bigger problems.

Finally, your compiler and environment likely has some guards for the times when you would like to introduce errors (scribbling), detect accesses to freed memory, and catch other related UB. You can also introduce similar diagnostics into your programs, often without affecting existing programs.

Multifoil answered 28/2, 2012 at 7:43 Comment(0)
L
1

Let me expand what you've already put into your question.

Here's what you've put into your question, in bullet-point form:


Setting pointers to NULL following delete is not universal good practice in C++. There are times when:

  • it is a good thing to do
  • and times when it is pointless and can hide errors.

However, there is no times when this is bad! You will not introduce more bugs by explicitly nulling it, you will not leak memory, you will not cause undefined behaviour to happen.

So, if in doubt, just null it.

Having said that, if you feel that you have to explicitly null some pointer, then to me this sounds like you haven't split up a method enough, and should look at the refactoring approach called "Extract method" to split up the method into separate parts.

Loewe answered 18/12, 2009 at 22:57 Comment(5)
I disagree with "there are no times when this is bad." Consider the amount of cutter this idiom introduces. You have a header being included in every unit that deletes something, and all those delete locations become just slightly less straight-forward.Paquette
There are times when it is bad. If someone tries to dereference your deleted-now-null pointer when they shouldn't, it probably won't crash and that bug is 'hidden.' If they dereference your deleted pointer which still has some random value in it, you are likely to notice and the bug will be easier to see.Locksmith
@Carson: My experience is quite the opposite: Dereferencing a nullptr will almost all ways crash the Application and can be caught by a debugger. Dereferencing a dangling pointer usually doesn't produce a problem right away, but will often just lead to incorrect results or other errors down the line.Poulos
@Poulos I completely agree, my views about this have changed substantially in the past ~6.5 yearsLocksmith
In terms of being a programmer, we were all someone else 6-7 years ago :) I'm not even sure I would've dared answering a C/C++ question today :)Loewe
D
1

There is always Dangling Pointers to worry about.

Dissipate answered 18/12, 2009 at 22:57 Comment(2)
Would it be so hard to put "dangling pointers" instead of "this"? :) At least it gives your answer some substance.Paquette
It's even the subject of a W3C QA tip, "Don't use 'click here' as link text": w3.org/QA/Tips/noClickHereLegere
J
1

Yes.

The only "harm" it can do is to introduce inefficiency (an unnecessary store operation) into your program - but this overhead will be insignificant in relation to the cost of allocating and freeing the block of memory in most cases.

If you don't do it, you will have some nasty pointer derefernce bugs one day.

I always use a macro for delete:

#define SAFEDELETE(ptr) { delete(ptr); ptr = NULL; }

(and similar for an array, free(), releasing handles)

You can also write "self delete" methods that take a reference to the calling code's pointer, so they force the calling code's pointer to NULL. For example, to delete a subtree of many objects:

static void TreeItem::DeleteSubtree(TreeItem *&rootObject)
{
    if (rootObject == NULL)
        return;

    rootObject->UnlinkFromParent();

    for (int i = 0; i < numChildren)
       DeleteSubtree(rootObject->child[i]);

    delete rootObject;
    rootObject = NULL;
}

edit

Yes, these techniques do violate some rules about use of macros (and yes, these days you could probably achieve the same result with templates) - but by using over many years I never ever accessed dead memory - one of the nastiest and most difficult and most time consuming to debug problems you can face. In practice over many years they have effectively eliminated a whjole class of bugs from every team I have introduced them on.

There are also many ways you could implement the above - I am just trying to illustrate the idea of forcing people to NULL a pointer if they delete an object, rather than providing a means for them to release the memory that does not NULL the caller's pointer.

Of course, the above example is just a step towards an auto-pointer. Which I didn't suggest because the OP was specifically asking about the case of not using an auto pointer.

Jobholder answered 18/12, 2009 at 23:11 Comment(4)
Macros are a bad idea, particularly when they look like normal functions. If you want to do this, use a templated function.Infundibuliform
Wow... I have never seen anything like anObject->Delete(anObject) invalidate the anObject pointer. That's just frightening. You should create a static method for this so that you are forced to do TreeItem::Delete(anObject) at the very least.Indigestive
Sorry, typed it in as a function rather than using the proper uppercase "this is a macro" form. Corrected. Also added a comment to explain myself better.Jobholder
And you're right, my quickly bashed out example was rubbish! Fixed :-). I was merely trying to think of a quick example to illustrate this idea: any code that deletes a pointer should ensure that the pointer is set to NULL, even if someone else (the caller) owns that pointer. So always pass in a reference to the pointer so it can be forced to NULL at the point of deletion.Jobholder
H
0

If you're going to reallocate the pointer before using it again (dereferencing it, passing it to a function, etc.), making the pointer NULL is just an extra operation. However, if you aren't sure whether it will be reallocated or not before it is used again, setting it to NULL is a good idea.

As many have said, it is of course much easier to just use smart pointers.

Edit: As Thomas Matthews said in this earlier answer, if a pointer is deleted in a destructor, there isn't any need to assign NULL to it since it won't be used again because the object is being destroyed already.

Hassle answered 18/12, 2009 at 23:2 Comment(0)
M
0

I can imagine setting a pointer to NULL after deleting it being useful in rare cases where there is a legitimate scenario of reusing it in a single function (or object). Otherwise it makes no sense - a pointer needs to point to something meaningful as long as it exists - period.

Mosely answered 19/12, 2009 at 17:14 Comment(0)
S
0

If the code does not belong to the most performance-critical part of your application, keep it simple and use a shared_ptr:

shared_ptr<Foo> p(new Foo);
//No more need to call delete

It performs reference counting and is thread-safe. You can find it in the tr1 (std::tr1 namespace, #include < memory >) or if your compiler does not provide it, get it from boost.

Shebat answered 19/12, 2009 at 19:10 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.