Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
System V shared memory APIs (2nd draft) #2314
base: master
Are you sure you want to change the base?
System V shared memory APIs (2nd draft) #2314
Changes from 22 commits
096cb21
072352c
adeaaa2
135488e
ac357e2
3f6a4c3
5047036
b428de0
07a6f33
ac33ca0
11aad71
d8db9fa
bd938e6
b9a5905
50916a4
f3e4771
9c71d97
5d0d37c
315eb31
9374dcf
bb07d5b
19c713d
17561e6
2b76711
94e4e11
aea8588
02b0de2
ed99763
b614d81
2629192
b7b8818
930f352
4bd41cf
2c714c2
6877712
b7f6a09
d07c9b8
5697ffd
3d70ae2
238fe62
0ff180a
f5f2f59
2803e31
35c2f4e
d05db61
ef1a839
e91efa3
d96d5ee
6aa8740
fd108e4
2fde2c5
e531e8b
fee5acb
52473ce
d526780
400f71e
d619c48
ca0c5f9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this
not(target_os = "android")
necessary asany(bsd, target_os = "linux")
does not include androidThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And it is always ok to not support all the platforms, we can add them incrementally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed the redundant
android
target. BSD is still not building and I don't really understand why. Something to do with macro? I'm not proficient with them yet.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OpenBSD and NetBSD CI failed because some missing constants in the libc crate, which means we need to add them before merging this PR if we want to have
#[cfg(bsd)
s supported.Let me take a closer look at this and probably give it a fix!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the last 9 bits of those
flg
arguments, Nix already has a type for it,nix::sys::stat::Mode
, so a new type is probably not necessary.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, wasn't aware of this type. This makes it depend on the
fs
module but I'm assuming it's fine?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is unfortunate, ideally, this
Mode
type should be available if#[cfg(any(feature="fs", feature="system_v")]
, doing this will require some extra work, so I think it is ok to makesystem_v
depend onfs
.