cpp: Fix evmc::result's move assignment operator #282
Conversation
chfast
force-pushed
the
fix_result_move
branch
2 times, most recently
from
140e700 to
152aaca
Compare
| @@ -39,25 +39,29 @@ class result : private evmc_result | |||
| /// Move constructor. | |||
| result(result&& other) noexcept : evmc_result{other} | |||
| { | |||
| // Disable releaser of the rvalue object. | |||
| other.release = nullptr; | |||
| other.release = nullptr; // Disable releasing of the rvalue object. | |||
There was a problem hiding this comment.
Should you do also other.output_data = nullptr to avoid dangling other?
There was a problem hiding this comment.
I was considering zeroing the other, but using an object that has been moved is an error anyway. I decided not to because there are other cases which will be hard to cover. Fortunately, some compilers warn you about using an object after std::move().
There was a problem hiding this comment.
For raw() there are no language rules though, and after calling raw() this contains dangling pointer
There was a problem hiding this comment.
There is a number of things we can do about it.
- This is already documented.
- The function is rather internal, needed to the Host interface. We might hide it by making it private and friend with the Host.
- We can zero the
otherinraw(). It might be better than the current but still not protecting against using the object afterraw()(it might actually hide the problem deeper) and that will set the.status_codetoEVMC_SUCCESS.
There was a problem hiding this comment.
You could zero out everything but status_code.
Anyway I don't think it's super-critical, having it documented might be enough.
(maybe I would name it release() to be similar to smart pointers)
There was a problem hiding this comment.
release()
I wanted that, but unfortunately evmc_result already has the release method. I wander how to rename it to make the usage more obvious. release_raw()? move_raw()? take_raw()?
There was a problem hiding this comment.
release_raw sounds better to me than raw
No description provided.