-
Notifications
You must be signed in to change notification settings - Fork 6
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
capi: make opt-in via 'capi' feature #59
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cyphar
force-pushed
the
capi-cbuild
branch
2 times, most recently
from
September 9, 2024 01:58
d1d6efb
to
3c07607
Compare
The whole module is #[cfg(test)]-ed out, but Rust 1.63 doesn't take into consideration whether or not a "pub fn" is actually exposed outside of the module so it's best to explicitly not export stuff we aren't using. The only test helper we use outside of src/tests is create_basic_root(). Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This lets us avoid users mistakenly thinking that our errors are errnos. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
We want to return an error if users specify an invalid mode. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This will allow us to mark the C API stuff as #[cfg(capi)] to let us build Rust crates without any C code. While we're at it, split out the CError stuff from the capi::ret and capi::utils modules to make it easier to see where things fit. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
There's no need to include the capi code for Rust crates. This also matches what cargo-c does, so we can easily switch without too many changes, though for now we can just keep using our own scripts. Part of making this opt-in means we only add the cdylib and staticlib crate types if building with that feature enabled. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
These functions all involve (at least) dereferencing pointers that callers must guarantee are safe. The only exception is pathrs_reopen which is technically safe to call from Rust code (bceause CBorrowedFd guarantees that the file descriptor is valid). Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This lets us ensure that the behaviour of Root/Handle and RootRef/HandleRef is consistent (because unfortunately we can't use Deref so technically the two types have separate implementations and so we could very easily end up with one of the types having a bug due to a copy-paste issue). By defining a trait, we can also prove a capi-backed trait impl that will let us test the C API from within our regular test suite. We can also make sure that methods are added to both types (and eventually the C API) because we cannot write a test using the new method without addding it to the trait and adding an implementation. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This will allow us to more easily make the tests work with a capi-backed ProcfsHandleImpl. The new macro is somewhat based on the root operations test macro. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This lets us finally get test coverage of our C API directly, as well as coverage information.The only thing to note is that we can't test resolve_partial because that is an internal resolver thing that isn't exposed to the C API. One thing to note is that the pathrs_reopen() tests are testing the ~O_CLOEXEC path right now even though that doesn't match the Rust API. We probably want to add fd flag tests to pathrs_reopen(), which will require correcting this issue. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Only thing left is to add some tests for fd flags (and fix the |
We need to guarantee to users that we are providing the flags they requested, so add some tests for that. In addition, add some extra checks to make sure that the O_CLOEXEC handling in the capi is correct. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
There's no need to include the capi code for Rust crates. This also
matches what cargo-c does, so we can easily switch without too many
changes, though for now we can just keep using our own scripts.
Part of making this opt-in means we only add the cdylib and staticlib
crate types if building with that feature enabled.
Fixes #53
Fixes #46
Signed-off-by: Aleksa Sarai cyphar@cyphar.com