0

I am trying to write a function that receives a pointer, uses it, and then makes it point to a new object. In order to do this, I am using a ptr-to-ptr. This is how I validate the ptr-to-ptr received by my function:

void modifyPtr(Obj ** ptrToPtr)
{
    if (*ptrToPtr == nullptr)
    {
        return;
    }
    else
    {
        // Do everything else!
    }
}

While writing this, I thought: what if a client passes the following to my function?

Obj ** ptrToPtr = nullptr;
modifyPtr(ptrToPtr);

In that case, my validation will be dangerous, because I will be dereferencing a nullptr. So, should I add an additional step of validation?

void modifyPtr(Obj ** ptrToPtr)
{
    if (ptrToPtr == nullptr)
    {
        return;
    }
    else if (*ptrToPtr == nullptr)
    {
        return;
    }
    else
    {
        // Do everything else!
    }
}

I have never seen validation like this one before, which is why I am hesitant.

Please be aware that I know one should avoid using raw pointers in C++. I am working with old code, and I find this problem interesting.

user3266738
  • 477
  • 2
  • 12
  • 2
    Your first validation is wrong, because you dereference `ptrToPtr` before checking if it is null. You probably don't need to check the dereferenced pointer for null since you are going to change it anyway (unless you need to do something with the old object). But, prefer using references instead of double-pointers, eg: `void modifyPtr(Obj* &Ptr)`, then the caller can't pass in a null reference – Remy Lebeau Jan 10 '18 at 01:13
  • 5
    `if (ptrToPtr && *ptrToPtr) { /* do Stuff */}` – Mooing Duck Jan 10 '18 at 01:15
  • @RemyLebeau you could literally just copy+paste that comment as an answer and it would be a good answer! – Tas Jan 10 '18 at 01:18
  • 2
    Unrelated: Nothing wrong with using raw pointers a lot of the time. Owning raw pointers is what you want to avoid. You want to automate the management to make it harder to miss the clean up, etc, but after that passing around the raw pointer may be exactly the right thing to do. But still prefer passing around references where possible. – user4581301 Jan 10 '18 at 01:19
  • Yeah @RemyLebeau, you should post it as an answer and I'll accept it. It's a great answer! Thank you :-) – user3266738 Jan 10 '18 at 01:22
  • @RemyLebeau it's not enough to change the function to accept a reference. You could still do `Obj ** ptrToPtr = nullptr; modifyPtr(*ptrToPtr);` and get some nice undefined behavior. – Mark Ransom Jan 10 '18 at 01:28
  • @MarkRansom: yes, but who in their right mind would actually do that? When you see something like `Obj* &ptr`, it is pretty apparent that the function expects the caller to do something like `Obj *ptr = ...; modifyPtr(ptr);` instead – Remy Lebeau Jan 10 '18 at 01:30
  • There is not much point in adding else clauses after doing a `return`. You either return or you don't. – Sid S Jan 10 '18 at 01:34
  • Prefer passing a smart pointer by reference, eg: [std::unique_ptr](http://en.cppreference.com/w/cpp/memory/unique_ptr). – Galik Jan 10 '18 at 01:46
  • @Galik if you do that you're restricting yourself from using any other pointer, e.g. `std::shared_ptr`. – Mark Ransom Jan 10 '18 at 02:31
  • @MarkRansom Indeed you are right. But ownership should be well defined so restricting what can own an object should be a benefit rather than a problem. And, after all, the alternative is not using smart pointers at all. – Galik Jan 10 '18 at 02:38
  • @Galik ownership of a pointer shouldn't be the concern of a function unless it's *changing* the ownership. Now in this particular example that might be appropriate, but I wouldn't consider it a general rule. – Mark Ransom Jan 10 '18 at 02:55

4 Answers4

1

Your first validation is wrong, because you dereference ptrToPtr before checking if it is null.

You probably don't need to check the dereferenced pointer for null since you are going to change it anyway (unless you need to do something with the old object).

However, you should prefer using references instead of double-pointers, eg:

void modifyPtr(Obj* &Ptr)

Then the caller can't pass in a null reference (without doing ugly hacks).

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • It's not an ugly hack, just an accident waiting to happen. `Obj **ptrToPtr = GetPP();` `Obj** GetPP() { if (some_condition) return nullptr; ... }`. I got burned by this once in real code so I'm extra sensitive to it now. If there's *any* possibility of having a null pointer, don't try to convert it to a reference and think you're safe. See https://stackoverflow.com/questions/57483/what-are-the-differences-between-a-pointer-variable-and-a-reference-variable-in/57656#57656 for my full exposition. – Mark Ransom Jan 10 '18 at 03:03
1

If you just return the new pointer instead of modifying the parameter, you can get away with just one level of indirection and simpler validation. The caller can decide if they want to reassign it immediately to the old value.

zneak
  • 134,922
  • 42
  • 253
  • 328
0

//use reference.

void modifyPtr(Obj *& ptrToPtr)
{
    if (ptrToPtr == nullptr)
    {
        return;
    }
    else
    {
        // Do everything else!
    }
}

Obj * ptrToPtr = nullptr;
modifyPtr(ptrToPtr);
love apple
  • 51
  • 4
0

In your first if you do not dereference anything. You just check if the passed pointer is null or not. So you do not have to worry. Just remove the * which i have not noticed on my mobile

0___________
  • 60,014
  • 4
  • 34
  • 74