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

Handle platforms where c_int is not i32 #69

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Handle platforms where c_int is not i32 #69

wants to merge 2 commits into from

Conversation

robin-nitrokey
Copy link
Member

This PR adds a CI job that tests compatibility with an avr target (16-bit) and fixes code that assumes that c_int is always i32.

Replaces:

One remaining problem is that some functions cast u32 return values to usize. In most cases that should be fine because the return value is limited by the length of the input buffer and thus fits into a usize. One notable exemption is seek. But let’s review and fix that separately.

Copy link
Contributor

@sosthene-nitrokey sosthene-nitrokey left a comment

Choose a reason for hiding this comment

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

LGTM but the test fails because it's missing the std source I think

@robin-nitrokey
Copy link
Member Author

Yeah, since rebasing onto main the CI fails, but the same commands worked before. Not sure why it fails now:

error: "/home/runner/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/Cargo.lock" does not exist, unable to build with the standard library, try:
        rustup component add rust-src --toolchain nightly-x86_64-unknown-linux-gnu

But we already installed rust-scr for that toolchain:

rustup toolchain install --profile minimal --component=rust-src nightly
info: syncing channel updates for 'nightly-x86_64-unknown-linux-gnu'
info: latest update on 2024-08-08, rust version 1.82.0-nightly (8b3870784 2024-08-07)
info: downloading component 'cargo'
info: downloading component 'rust-src'
info: downloading component 'rust-std'
info: downloading component 'rustc'
info: installing component 'cargo'
info: installing component 'rust-src'
info: installing component 'rust-std'
info: installing component 'rustc'

  nightly-x86_64-unknown-linux-gnu installed - rustc 1.82.0-nightly (8b3870784 2024-08-07)

robin-nitrokey added a commit that referenced this pull request Aug 15, 2024
This patch applies those changes from
#69 that affect the core
crate.
Some functions assume that c_int is always i32.  This is not true for
all platforms.  This patch adapts those functions to handle those cases.

Some littlefs functions return an error code (c_int) from functions
returning i32.  In these cases, error codes are truncated to c_int::MIN.
@robin-nitrokey
Copy link
Member Author

And another rebase fixed it. 🤔

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.

3 participants