2

It's my understanding that in the case of

Ptr* p = new Ptr();

that I should delete p; p = 0. What's not clear to me is what to do in the case of:

Ptr* p = obj.GetPtr()

I read that I should not delete p since this could lead to adverse effects. What if the function GetPtr() itself news some pointers but doesn't delete them?

vinpa
  • 51
  • 1
  • 9
    It's up to the function's documentation to tell you what to do. Nobody can guess what the right course of action is. Perhaps a suggestive function name can give you a hint. This is another reason owning raw pointers are generally bad. – François Andrieux Nov 30 '17 at 19:55
  • 8
    When Hamlet writes C++ – Vittorio Romeo Nov 30 '17 at 19:56
  • Every executed `new` needs a corresponding executed `delete`, lest you leak memory. It's generally not a good idea to null a pointer after `delete`. The requirements of a function depends on the function. – Cheers and hth. - Alf Nov 30 '17 at 19:56
  • Instead of manual `new` and `delete`, use smart pointers. Instead of smart pointers, use standard or 3rd party collections, where applicable. E.g., use `std::string`, not `char*`; use `std::vector` for dynamic arrays, and so on. – Cheers and hth. - Alf Nov 30 '17 at 19:58
  • You should not set a pointer to zero after calling `delete` on it unless there is some specific reason to do so. This is an anti-pattern. – David Schwartz Nov 30 '17 at 20:01
  • 1
    Whether tis nobler in the mind to suffer the stars and arrows of dangling pointers – M.M Nov 30 '17 at 20:03
  • As @M.M says, won't I have a "dangling pointer" if I don't `NULL` it after the delete? Why is it generally not a good idea to `NULL`? – vinpa Nov 30 '17 at 20:06
  • @vinpa Dangling pointers refers to when you _use_ a pointer after deleting it. – Passer By Nov 30 '17 at 20:06
  • 1
    @vinpa Several reasons. For one, a dangling pointer conveys the information that the pointer used to refer to an object. You cannot destroy that information unless you know, in that specific instance, that it is safe to do so. For another, it tends to lead to code that assumes that a non-NULL pointer is safe to dereference, which it isn't in general if pointers are passed around. (Consider two pointers that point to the same object and `delete` is called on one of them. The other is non-NULL, but it is not safe to access. You can't use "it's not NULL" to mean it's safe to access.) – David Schwartz Nov 30 '17 at 20:07
  • 1
    More clearly: Either no code accesses that value or some code accesses that value. If no code accesses that value, then setting the value to zero accomplishes nothing. If some code does access that value, then it cares what the value is and it matters what it is. So setting it to NULL may help that code or may break that code depending on whether the code needs to know whether the pointer now points to a valid object or previously pointed to a valid object. So you need to carefully decide whether, in this case, the pointer should be set to 0 or not. So there can't be a general policy,. – David Schwartz Nov 30 '17 at 20:12
  • @DavidSchwartz Your explanation has a non-sequitur. If there is some code that accesses a deleted pointer value then the program has undefined behaviour. So by definition it cannot break the program to set the pointer to null instead. (Programs with UB are already broken) – M.M Nov 30 '17 at 20:18
  • The only real argument against nulling a deleted pointer is that it might waste CPU time but some feel that's a small price to pay for avoiding bugs associated with using invalid pointers – M.M Nov 30 '17 at 20:20
  • @M.M I'll check the standard again, but the I'm pretty sure the last time I checked the conclusion was that it was perfectly safe to compare it to nullptr. The argument against it is that it either does nothing or changes behavior and if it changes behavior, you need to make sure it changes from the behavior you don't want to the behavior you want and not the reverse. So it requires a decision based on the specifics of that particular case. (It also creates the bad habit of believing that if a pointer is non-NULL it must refer to a valid object.) – David Schwartz Nov 30 '17 at 20:26
  • @DavidSchwartz in fact it has implementation-defined behaviour since C++14 (which may include generating a runtime fault). So you may have a valid point, if we're working on an implementation that doesn't generate a fault and we're not trying to write portable code – M.M Nov 30 '17 at 20:28

1 Answers1

10

In Modern C++, the convention is that raw pointers are non-owning pointers - therefore code that you write and libraries that follow modern convention should not require raw pointers to be deleted.

In order to express ownership of an heap-allocated object, smart pointers are used instead (std::unique_ptr and std::shared_ptr). These pointers automatically invoke delete for you when appropriate.

Relevant core guideline: "Never transfer ownership by a raw pointer (T*) or reference (T&)".

In your particular case:

What's not clear to me is what to do in the case of:

Ptr* p = obj.GetPtr()

  1. Assume that p is non-owning, as it is a raw pointer.

  2. If you're using a legacy library, check the documentation of obj.GetPtr(). If it needs to be deleted, the documentation should mention it.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
Vittorio Romeo
  • 90,666
  • 33
  • 258
  • 416
  • 1
    And the larger lesson is that any time you have something that refers to an object (whether a raw pointer, a reference, a smart pointer, or whatever) there should always be a clear understanding of how the object's lifetime is managed. – David Schwartz Nov 30 '17 at 20:03
  • In case (2) you may want to use `std::unique_ptr p = obj.GetPtr();` – M.M Nov 30 '17 at 20:04
  • Romeo: can you please elaborate a bit about **ownership**? – vinpa Nov 30 '17 at 20:08
  • @M.M That won't compile. – Passer By Nov 30 '17 at 20:08
  • 1
    @M.M: `std::unique_ptr p(obj.GetPtr());`, but again, only if the caller is expected to `delete` the pointer that `GetPtr()` returns – Remy Lebeau Nov 30 '17 at 20:45