Skip to content
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

uapi: Use libc for syscall constants #18

Closed
wants to merge 1 commit into from

Conversation

cd-work
Copy link
Contributor

@cd-work cd-work commented Oct 4, 2022

This patch switches from internally defined system call constants for
the landlock system calls to the ones defined by libc.

Besides removing unnecessary code already provided by the latest libc,
this also adds support for additional architectures like ARM since the
existing constants were limited to x86_64.

@cd-work cd-work force-pushed the libc_constants branch 2 times, most recently from a55e26e to 2f81ca1 Compare October 4, 2022 15:48
@l0kod
Copy link
Member

l0kod commented Oct 6, 2022

Thanks! Can you please remove the Cargo.lock from the commit?

@cd-work
Copy link
Contributor Author

cd-work commented Oct 6, 2022

Thanks! Can you please remove the Cargo.lock from the commit?

Sure, want me to add it to the .gitignore to match what Rust generates by default for libraries?

@l0kod
Copy link
Member

l0kod commented Oct 6, 2022

Please just squash the commits.

Cargo.toml Outdated
@@ -16,7 +16,7 @@ readme = "README.md"

[dependencies]
enumflags2 = "0.7"
libc = "0.2"
libc = "0.2.134"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it the earlier libc version where the Landlock syscalls have been added? It's a pity that the libc crate is using such versioning because we cannot have any guarantee that the used crate will have these flags… This is unfortunate but I still think this commit is worth it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The earliest version is actually 0.2.133, I've changed it to make support a tiny bit better even.

It's a pity that the libc crate is using such versioning because we cannot have any guarantee that the used crate will have these flags

I'm not sure what you mean about this. The negotiation for versions should only ever go up, so it shouldn't be possible to pull landlock and end up with a version under 0.2.133 (after my latest change). So I think we can guarantee that the used crate will have these flags?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, I was thinking about the 0.* exception but it is unrelated.

@l0kod
Copy link
Member

l0kod commented Oct 6, 2022

You actually pushed rust-lang/libc#2904, thanks!

@l0kod
Copy link
Member

l0kod commented Oct 6, 2022

If you want to keep your commit signature, please prefix the subject with "uapi: ", otherwise I'll do it myself.

This patch switches from internally defined system call constants for
the landlock system calls to the ones defined by `libc`.

Besides removing unnecessary code already provided by the latest libc,
this also adds support for additional architectures like ARM since the
existing constants were limited to x86_64.

Signed-off-by: Christian Duerr <chris.durr@phylum.io>
@cd-work cd-work changed the title Use libc for syscall constants uapi: Use libc for syscall constants Oct 6, 2022
@l0kod l0kod closed this in 0a9edc6 Oct 7, 2022
@l0kod
Copy link
Member

l0kod commented Oct 7, 2022

Thanks!

I removed the Cargo.lock file.

@cd-work cd-work deleted the libc_constants branch October 7, 2022 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants