Created on 2023-02-27.00:00:00 last changed 13 months ago
Proposed resolution:
This wording is relative to N4928.
Modify [inout.ptr.t] as indicated:
~inout_ptr_t();-9- Let SP be POINTER_OF_OR(Smart, Pointer) ([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:
(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;
(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;
(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;
(11.4) — otherwise, the program is ill-formed.
[ 2023-11-11 Approved at November 2023 meeting in Kona. Status changed: Voting → WP. ]
[ Varna 2023-06-16; Move to Ready ]
[ 2023-03-22; Reflector poll ]
Set priority to 2 after reflector poll.
inout_ptr seems useful for two purposes:
Using smart pointers with C-style APIs.
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:Constructor: If the user's pointer is a smart pointer, perform a "release" operation.
(C-style API executes)
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: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.
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.
History | |||
---|---|---|---|
Date | User | Action | Args |
2023-11-13 14:08:10 | admin | set | messages: + msg13841 |
2023-11-13 14:08:10 | admin | set | status: voting -> wp |
2023-11-07 21:41:54 | admin | set | status: ready -> voting |
2023-06-16 09:32:19 | admin | set | messages: + msg13641 |
2023-06-16 09:32:19 | admin | set | status: new -> ready |
2023-03-22 22:40:39 | admin | set | messages: + msg13482 |
2023-03-04 12:14:40 | admin | set | messages: + msg13442 |
2023-02-27 00:00:00 | admin | create |