Expose OpaqueIpcMessage as IpcMessage
#362
Conversation
|
This will need #364 to work properly. |
Cargo.toml
Outdated
| @@ -20,6 +20,7 @@ name = "ipc_receiver_set" | |||
| harness = false | |||
|
|
|||
| [features] | |||
| default = ["async"] | |||
There was a problem hiding this comment.
Whoops, this was left over from local testing. I've removed it.
src/ipc.rs
Outdated
| let _ = mem::replace( | ||
| &mut *os_ipc_shared_memory_regions_for_deserialization.borrow_mut(), | ||
| &mut self.os_ipc_shared_memory_regions, | ||
| old_ipc_shared_memory_regions_for_deserialization, |
There was a problem hiding this comment.
I think this would be equivalent to (and more clear as):
*os_ipc_shared_memory_regions_for_deserialization.borrow_mut() = old_ipc_shared_memory_regions_for_deserialization;
mrobinson
force-pushed
the
make-recv-friendlier
branch
from
a8017a1 to
14c4add
Compare
This makes the API a bit friendlier and future-proof. Instead of returning a tuple from raw `recv()` return the `IpcMessage` struct that is used internally in the crate. In addition, this is used to pass data from the platform layer -- removing clippy warnings about complex data types. One upside to this is that `Option<IpcSharedMemory>` is turned into `IpcSharedMemory` in some places where having a `None` value shouldn't be possible. This simplification removes about 300 lines of code.
mrobinson
force-pushed
the
make-recv-friendlier
branch
from
14c4add to
1f231e4
Compare
|
Thanks for the review! |
| impl OpaqueIpcMessage { | ||
| fn new( | ||
| impl IpcMessage { | ||
| pub(crate) fn new( |
There was a problem hiding this comment.
I'm curious if there is reason for not marking this as pub. The struct only has pub fields, so it doesn't seem like we are gaining much in terms of encapsulation.
There was a problem hiding this comment.
Fair enough. I think mainly this wasn't marked as pub because it isn't expected that a user of ipc-channel would ever need to create this data structure. pub(crate) is useful in this case because the compiler can determine if the function ever becomes unused.
There was a problem hiding this comment.
because it isn't expected that a user of ipc-channel would ever need to create this data structure
Should we then remove the 'pub' from the fields? Having them public means it will become part of our API in the next release and OpaqueIpcMessage only had private fields. I'm happy to do the change.
There was a problem hiding this comment.
I am not opposed to making the fields private again.
This makes the API a bit friendlier and future-proof. Instead of
returning a tuple from raw
recv()return theIpcMessagestruct thatis used internally in the crate. In addition, this is used to pass data
from the platform layer -- removing clippy warnings about complex data
types. One upside to this is that
Option<IpcSharedMemory>is turnedinto
IpcSharedMemoryin some places where having aNonevalueshouldn't be possible.
This simplification removes about 300 lines of code.