delete pointer argument in functions

[hebrerillo]

I would like to ask you something that I think it could be considered a bad practice.

Let's think of a function which takes a pointer of type 'MyType' as an argument. The function deletes the pointer. It would be something like this

void workWithPointer(MyType *p)
{
    //do some stuff with p
    delete p; //this does not delete de pointer
    p = NULL;
}

I am working in a project and I found code like this. But as you already know, the pointer 'p' is not actually deleted. The memory is still there.

When the function deletes the pointer argument. Would not be a better approach to work with double indirection pointers?

Something like this:

void workWithPointer(MyType **p)
{
    //do some stuff with *p
    delete *p; //this deletes de pointer
    *p = NULL;
}

What are your thoughts about this?

Thanks a lot!

[MikeyBoy]

So, basically, your problem with the first version is that, while it sets the local pointer p to be NULL, the pointer in the calling code isn't automatically set to NULL, and so could accidentally be used to try and access memory that is no longer valid? Is that correct?

If I've understood you correctly, that's a valid criticism, and your proposed second version fixes that.

A more elegant version would be to pass in a reference to the pointer:

void workWithPointer(MyType*& p)
{
    //do some stuff with p
    delete p; //this deletes de pointer
    p = NULL;
}

The comments you've written on line 4 in each of your versions don't make much sense. In both cases, line 4 deletes the object that the pointer is pointing to. Nothing deletes the pointer itself, in either version. The only difference between the two is in whether the new value of the pointer (NULL) is passed back to the calling code.

I suspect you've misunderstood something about pointers and/or dynamic memory management, but I'm not sure what, exactly.

[gunnerfunner]

#include <iostream>

void workWithPointer(MyType *p)
{
    //do some stuff with p
    delete p; //this does not delete the pointer variable - correct, only the object managed by p ...
    //... but even delete p not required if object managed by p was not created via the new operator i.e. on the heap
    p = NULL;//use nullptr instead
    //http://stackoverflow.com/questions/20509734/null-vs-nullptr-why-was-it-replaced
    //http://stackoverflow.com/questions/13816385/what-are-the-advantages-of-using-nullptr
} <-//variable p exists only until the closing brace of the function and will be automatically deleted once the function goes out of scope
//delete is not required even for pointers managing objects created with new operator if these pointers are smart pointers:
//http://umich.edu/~eecs381/handouts/C++11_smart_ptrs.pdf

int main()
{
   int* p = new int(7);
   //p = nullptr; //OK
   //*p = NULL;//|warning: converting to non-pointer type 'int' from NULL [-Wconversion-null]|
   *p = nullptr;//error: cannot convert 'std::nullptr_t' to 'int' in assignment|
  delete p;
}

[Ericool]

What is p an array? If so delete[]

[Cubbi]

What are your thoughts about this?

I think setting a deleted pointer to null is a very bad idea.

The purpose of this maneuver is to make it possible to execute delete on the same pointer twice (deleting a null pointer has no effect), which means the program has no clue what has or hasn't been allocated. That needs to be fixed, not enabled by nulling out pointers.

In any case, in post-2011 C++, there is never a need to write delete p; to begin with; we have unique_ptr which tracks ownership for you.

(and yes, if you really want to do that, MikeyBoy's MyType*& p is the way to go)

[dhayden]

Let's think of a function which takes a pointer of type 'MyType' as an argument. The function deletes the pointer.

It's usually good practice for the thing that allocated the memory to also delete it. So personally, I would rarely have a function delete a pointer that was passed into it.

The key thing when doing your own memory management is to be crystal clear on who owns the pointer (really the data that it points to). The owner is responsible for deleting the data. Make a decision. Document it in the code and stick to it. If your ownership policy makes sense for the function to delete that pointer then go for it.
Just be sure to make that crystal clear in the comments.

Cubbi writes:

I think setting a deleted pointer to null is a very bad idea.

The purpose of this maneuver is to make it possible to execute delete on the same pointer twice

I agree that it's a bad idea to set a pointer to null so it can be deleted twice, but that's not why I set deleted pointers to null. I do it to "poison" the pointer so that any mistaken use of it later will most likely crash the program. Thinking about delete nullptr though, maybe it would be better to set deleted pointers to 1 instead. That would likely cause a crash if someone tried to delete it again.

[jonnin]

just use auto_ptr if you are feeling tempted to get fancy with this project. Trying to do it yourself is just going to end up with exactly the same functionality.

[Cubbi]

dhayden wrote:

to "poison" the pointer so that any mistaken use of it later

right, that's a little less bad, I forgot about this intent - haven't really worked with such code.

[hebrerillo]

Hi everyone! Thanks for your answers.

MickeyBoy, yes you are right. I did not explain myself very good (sorry for my poor English).

What I wanted to achieve with this code is to set the pointer to null within the function 'workWithPointer'. So later on, some piece of code could check for that pointer to be null. Let's see an example:

void workWithPointer(MyType **p)
{
    //do some stuff with *p
    delete *p; //this deletes de pointer
    *p = NULL;//sets the pointer to point to NULL (or nullptr)
}


int
main()
{
    MyType * a = new MyType(1);
    
    //do some stuff with a
    if(a !== NULL)
    {
        a->myfunc();
    }
}

The code you provided was very useful!! I did not know you could pass a reference to a pointer. Thanks a lot!