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

capi: make opt-in via 'capi' feature #59

Merged
merged 11 commits into from
Sep 10, 2024
Merged

Conversation

cyphar
Copy link
Member

@cyphar cyphar commented Sep 7, 2024

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

@cyphar cyphar force-pushed the capi-cbuild branch 2 times, most recently from d1d6efb to 3c07607 Compare September 9, 2024 01:58
@cyphar cyphar changed the title capi: switch to cargo-c capi: make opt-in via 'capi' feature Sep 9, 2024
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>
@cyphar
Copy link
Member Author

cyphar commented Sep 10, 2024

Only thing left is to add some tests for fd flags (and fix the O_CLOEXEC disparity between the two implementations when it comes to the tests).

@cyphar
Copy link
Member Author

cyphar commented Sep 10, 2024

screenshot-2024-09-11T013408-ZrdgVuesp4

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>
@cyphar cyphar marked this pull request as ready for review September 10, 2024 16:59
@cyphar cyphar merged commit a1f8e39 into openSUSE:main Sep 10, 2024
15 checks passed
@cyphar cyphar deleted the capi-cbuild branch September 11, 2024 05:37
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.

cffi: we should probably not include capi stuff in the standard crate tests: add test coverage for C API?
1 participant