Add support for shared memory operations for solaris/illumos #1584
Conversation
This is needed because Firefox now uses slice-deque rust crate.
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @gnzlbg (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
@bors: r+ |
|
📌 Commit a7c0dcf has been approved by |
|
@bors p=100 |
Add support for shared memory operations for solaris/illumos This is needed because Firefox now uses slice-deque rust crate.
|
💔 Test failed - checks-cirrus-freebsd-11 |
|
@bors: retry |
Add support for shared memory operations for solaris/illumos This is needed because Firefox now uses slice-deque rust crate.
|
💔 Test failed - status-azure |
There was a problem hiding this comment.
shmid_ds is okay for 64 bit CPUs. For 32 bit there would have to be additional padding. But I do not think that Rust supports Solaris on 32 bit.
IPC_ALLOC, IPC_CREAT, IPC_EXCL, and IPC_NOWAIT take different values on illumos.
Everything else seems to be okay.
| pub struct shmid_ds { | ||
| pub shm_perm: ipc_perm, | ||
| pub shm_segsz: ::size_t, | ||
| pub shm_flags: ::uintptr_t, |
There was a problem hiding this comment.
There is no shm_flags in illumos. This is also at the location where shm_amp would be for illumos. In addition, shm_gransize and shm_allocated are not defined on illumos, and the padding appears to be incorrect for illumos.
There was a problem hiding this comment.
https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/sys/shm.h#L88-L115 shows the illumos definition (as @bgermann said, currently there is no 32-bit support for rust on illumos, so it's probably not necessary to add the 32-bit versions -- if that were to ever happen, it'd probably better be done as its own change since many more things would need to be updated).
There was a problem hiding this comment.
The IPC_* values are ok -- for likely historical reasons lost to time, the values are defined in octal on illumos (but appear to match what you have)
There was a problem hiding this comment.
Oh, yes, so forget about that.
|
Is there anything that @psumbera needs to update here or is this ready? |
|
The shmid_ds struct is wrong for illumos. I cannot say if it's correct for Solaris or not. At minimum it either needs to be guarded so it's only used with the solaris target, or updated so it's correct on both. |
|
Looks good now. Thanks for the update! |
|
@bors: r+ |
|
📌 Commit 3c1b0ee has been approved by |
Add support for shared memory operations for solaris/illumos This is needed because Firefox now uses slice-deque rust crate.
|
💔 Test failed - status-azure |
|
@bors: r+ |
|
📌 Commit 412dd4e has been approved by |
Add support for shared memory operations for solaris/illumos This is needed because Firefox now uses slice-deque rust crate.
|
💔 Test failed - status-azure |
|
@bors: retry |
Add support for shared memory operations for solaris/illumos This is needed because Firefox now uses slice-deque rust crate.
|
☀️ Test successful - checks-cirrus-freebsd-10, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, status-azure |
This is needed because Firefox now uses slice-deque rust crate.