3897. inout_ptr will not update raw pointer to 0

Section: 20.3.4.3 [inout.ptr.t] Status: WP Submitter: Doug Cook Opened: 2023-02-27 Last modified: 2023-11-22

Priority: 2

View all other issues in [inout.ptr.t].

View all issues with WP status.

Discussion:

inout_ptr seems useful for two purposes:

1. Using smart pointers with C-style APIs.

2. Annotating raw pointers for use with C-style APIs.

Unfortunately, as presently specified, it is not safe for developers to use inout_ptr for the second purpose. It is not safe to change code from

void* raw_ptr1;
InitSomething(&raw_ptr1);
UpdateSomething(&raw_ptr1); // In some cases may set raw_ptr1 = nullptr.
CleanupSomething(raw_ptr1);

to

void* raw_ptr2;
InitSomething(std::out_ptr(raw_ptr2));
UpdateSomething(std::inout_ptr(raw_ptr2)); // May leave dangling pointer
CleanupSomething(raw_ptr2);                // Possible double-delete

In the case where UpdateSomething would set raw_ptr1 = nullptr, the currently-specified inout_ptr implementation will leave raw_ptr2 at its old value. This would likely lead to a double-delete in CleanupSomething.

The issue occurs because inout_ptr is specified as follows:

1. Constructor: If the user's pointer is a smart pointer, perform a "release" operation.

2. (C-style API executes)

3. If the C-style API returns a non-NULL pointer, propagate the returned value to the user's pointer.

If the user's pointer is not a smart pointer, no "release" operation occurs, and if the C-style API returns a NULL pointer, no propagation of the NULL occurs. We're left with a dangling raw pointer which is different from the original behavior using &.

I see two potential solutions:

1. Make the "release" operation unconditional (i.e. it applies to both smart and raw pointers). For raw pointers, define the "release" operation as setting the raw pointer to nullptr.

2. Make the return value propagation unconditional for raw pointers.

Solution #2 seems likely to lead to more optimal code as it avoids an unnecessary branch.

[2023-03-22; Reflector poll]

Set priority to 2 after reflector poll.

[Varna 2023-06-16; Move to Ready]

[2023-11-11 Approved at November 2023 meeting in Kona. Status changed: Voting → WP.]

Proposed resolution:

This wording is relative to N4928.

1. Modify 20.3.4.3 [inout.ptr.t] as indicated:

   ~inout_ptr_t();
   

   -9- Let SP be POINTER_OF_OR(Smart, Pointer) (20.2.1 [memory.general]).

   -10- Let release-statement be s.release(); if an implementation does not call s.release() in the constructor. Otherwise, it is empty.

   -11- Effects: Equivalent to:

   1. (11.1) —

      if (p) {
        apply([&](auto&&... args) {
          s = Smart( static_cast<SP>(p), std::forward<Args>(args)...); }, std::move(a));
      }
      

      if is_pointer_v<Smart> is true;

   2. (11.2) — otherwise,

      release-statement;
      if (p) {
        apply([&](auto&&... args) {
          s.reset(static_cast<SP>(p), std::forward<Args>(args)...); }, std::move(a));
      }
      

      if the expression s.reset(static_cast<SP>(p), std::forward<Args>(args)...) is well-formed;

   3. (11.3) — otherwise,

      release-statement;
      if (p) {
        apply([&](auto&&... args) {
          s = Smart(static_cast<SP>(p), std::forward<Args>(args)...); }, std::move(a));
      }
      

      if is_constructible_v<Smart, SP, Args...> is true;

   4. (11.4) — otherwise, the program is ill-formed.