Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upOsIpcOneShotServer cleanups #114
Conversation
Make `OsIpcOneShotServer.accept()` take `mut self` (just like the `macos` implementation) instead of `&self`, thus avoiding the need for interior mutability. Unifying the interface is an extra bonus...
|
As usual, this is not tested on MacOS... Let the CI bots speak :-) |
|
Gah, so much for CI bots :-( Can we try that again please?... |
|
@bors-servo r+ |
|
|
OsIpcOneShotServer cleanups A pair of cleanups removing unneeded wrappers inside `OsIpcOneShotServer`; plus a not truly related (apart from touching the same code) minor error handling fix in the `inprocess` implementation, which I cheated in here to avoid dependant PRs...
|
|
|
No log for the failing build, that sucks :( |
|
cc @larsbergstrom (read backlog) |
|
I clicked restart so we can check it out again. If that doesn't generate a log, we will need to send mail to support@travis-ci.com to get help. |
|
@larsbergstrom now suddenly it does show the log for the original CI run alright... Weird, but good enough for me :-) |
This implementation wrapped `OsIpcOneShotServer.receiver` in an `Option<>` for no reason: between creation in `new()` and removal at the end of `accept()`, there is no place where the value actually needs to be unset. The field was only being replaced temporarily with None within `accept()`, just to be dropped along with the rest of `OsIpcOneShotServer` a moment later at the end of the method... Since we have ownership of the value within `accept()`, and no `drop()` is defined for `OsIpcOneShotServer`, we can just move the `receiver` out instead. Note: since we no longer explicitly unset `self.receiver`, there is no need for `self` to be mutable anymore.
Just like in the other implementations, there is no need to panic here.
Unlike the `inprocess` back-end, this one indeed does need to unset `OsIpcOneShotServer.receiver` within `accept()`, because this variant of `OsIpcOneShotServer` has a `drop()` implementation -- so we can't just move out the `receiver`. Still, we do not need to explicitly wrap the field in an `Option<>`, since `OsIpcReceiver` can be unset internally with `consume()` -- which returns a new `OsIpcReceiver` instance for the port, while unsetting the original one. This appeases `OsIpcOneShotServer.drop()`, while making sure `OsIpcReceiver.drop()` won't free the actual port.
antrik
force-pushed the
antrik:cleanup-OsIpcOneShotServer
branch
from
6c94c38
to
5a04257
|
Different approach for the |
|
Yay! |
|
@bors-servo: r+ |
|
|
OsIpcOneShotServer cleanups A pair of cleanups removing unneeded wrappers inside `OsIpcOneShotServer`; plus a not truly related (apart from touching the same code) minor error handling fix in the `inprocess` implementation, which I cheated in here to avoid dependant PRs...
|
|
antrik commentedNov 1, 2016
A pair of cleanups removing unneeded wrappers inside
OsIpcOneShotServer; plus a not truly related (apart from touching the same code) minor error handling fix in theinprocessimplementation, which I cheated in here to avoid dependant PRs...