SHM mutation #328
SHM mutation #328
Conversation
yellowhatter
commented
Dec 17, 2024
yellowhatter
commented
- mutable access to SHM example
- added necessary mutable accessors here and there
- added necessary mutable accessors here and there
yellowhatter
added
enhancement
| #if defined(ZENOHCXX_ZENOHC) | ||
| /// @brief The payload of this error. | ||
| /// @return error payload. | ||
| Bytes& get_payload() { |
There was a problem hiding this comment.
today there is no way to get mutable ReplyErr or Sample from reply, so this method is useless as it is.
There was a problem hiding this comment.
Then we should add this. SHM buffer may reside literally everywhere where ZBytes is
There was a problem hiding this comment.
That is the problem. We can not add these methods, because this would allow user to apply std::move to mutable Sample or ReplyError which would lead to their destruction. Since on zenoh-c level calling destructors is only possible on owned objects and sample/error are stored as loaned inside reply this would lead to UB.
So it is ok to merge this pr as is, but adding mutable access to sample/reply_err from Reply would require first resolving eclipse-zenoh/zenoh-c#718.
There was a problem hiding this comment.
Made mutable access to sample/reply_err from Reply private for a while.
| @@ -433,14 +433,14 @@ class Session : public Owned<::z_owned_session_t> { | |||
| SubscriberOptions&& options = SubscriberOptions::create_default(), | |||
| ZResult* err = nullptr) const { | |||
| static_assert( | |||
| std::is_invocable_r<void, C, const Sample&>::value, | |||
| std::is_invocable_r<void, C, Sample&>::value, | |||
There was a problem hiding this comment.
Similar fixes need to be done for every subscriber/queryable and get function :(
|
@milyin I think, this PR really requires eclipse-zenoh/zenoh-c#718 to be resolved first. |
Conflicts: include/zenoh/api/ext/querying_subscriber.hxx
