Skip to content

nuttx: clean up module#5245

Open
dybucc wants to merge 2 commits into
rust-lang:mainfrom
dybucc:nuttx-cleanup
Open

nuttx: clean up module#5245
dybucc wants to merge 2 commits into
rust-lang:mainfrom
dybucc:nuttx-cleanup

Conversation

@dybucc

@dybucc dybucc commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Description

This PR consists of an overall clean up of the NuttX module. It adds missing types, fixes one type definition, replaces equivalent with mirrored definitions (fixed-width types with "variable" C types,) and provides correct support for some build-time configuration options.

The build-time configuration options correspond with CONFIG_SMALL_MEMORY and CONFIG_FS_LARGEFILE. The former enables types with pointer bit width (size_t and ssize_t) to be narrower than required in the target. It is enabled by default in 16-bit machine word targets but can also be enabled in other targets. The latter option enables support for LFS bindings. The default under all targets is to expose 32-bit types and no suffixed 64-bit types. It is only if the above option is set that (1) suffixed types are exposed and (2) unsuffixed types are made 64-bits wide. I have decided against exposing the suffixed types because it's redundant.

Sources

Checklist

  • Relevant tests in libc-test/semver have been updated
  • No placeholder or unstable values like *LAST or *MAX are included (see #3131)
  • Tested locally (cd libc-test && cargo test --target mytarget); especially relevant for platforms that may not be checked in CI

@rustbot label +stable-nominated

@rustbot rustbot added S-waiting-on-review stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Jul 3, 2026
Comment thread src/unix/nuttx/mod.rs Outdated
Comment thread src/unix/nuttx/mod.rs Outdated
Comment thread src/unix/nuttx/mod.rs Outdated
Comment thread src/unix/nuttx/mod.rs Outdated
Comment thread src/unix/nuttx/mod.rs Outdated
pub type key_t = u32;
pub type nlink_t = u16;
pub type ino_t = u16;
pub type ino_t = u32;

@tgross35 tgross35 Jul 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"nuttx: fix ino_t definition" this happened just last week apache/nuttx@50377d0, I have no clue what the release/support schedule looks like but we should probably hold off a bit. Or at least hear from the maintainers.

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

just pinged the maintainers.

Comment thread src/unix/nuttx/mod.rs Outdated
Comment thread src/unix/nuttx/mod.rs
pub type locale_t = *mut i8;
pub type mode_t = u32;
pub type nfds_t = u32;
pub type off_t = i64;

@tgross35 tgross35 Jul 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"nuttx: provide correct lfs support": It looks like we were already supporting CONFIG_FS_LARGEFILE - why the change?

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we were not correctly supporting that configuration. without it enabled, all targets get 32-bit file offset types. with it enabled, they all get 64-bit file offset types.

it would likely come off as a surprise if somebody didn't have CONFIG_FS_LARGEFILE enabled but found that our types are 64-bits either way.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I mean we were only supporting CONFIG_FS_LARGEFILE, right? That's completely fine, every platform has dozens of ways you could configure it that make something break for Rust.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I mean we were only supporting CONFIG_FS_LARGEFILE, right? That's completely fine, every platform has dozens of ways you could configure it that make something break for Rust.

Yes, it is unlikely to support arbitrary configurations of the NuttX ABI on the Rust side, so we must restrict the configuration options that affect fundamental data type definitions and only support the most commonly used combinations.

Comment thread src/unix/nuttx/mod.rs Outdated
}

cfg_if! {
if #[cfg(nuttx_small_mm)] {

@tgross35 tgross35 Jul 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"nuttx: expose cfg for small memory model" similarly, I don't think there is any need to add this - it's extra configuration complexity in libc and has no known users. This would probably have to be a separate target in rustc, anyway.

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed.

@rustbot

This comment has been minimized.

@dybucc

dybucc commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

@no1wudi, you think we can already have an ino_t definition that matches upstream? there was a recent change there and this pr includes it, but maybe that's a bit too early to make changes.

@rustbot ready

Comment thread src/unix/nuttx/mod.rs Outdated
// types are themselves 64-bits wide. We choose to expose ony the latter.
cfg_if! {
if #[cfg(gnu_file_offset_bits64)] {
pub type fsblkcnt_t = u64;

@no1wudi no1wudi Jul 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There are too many configuration options inside Nuttx; it would be better to only support specific projects. For example, FS_LARGEFS must be enabled; otherwise, compilation becomes overly difficult.

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done. thanks for getting back.

Comment thread src/unix/nuttx/mod.rs Outdated
dybucc added 2 commits July 8, 2026 18:18
fix signedness of wchar_t under arm and aarch64. this was not correctly
handled as this type will likely be defined in terms of predefined
compiler macros [^1]. these macros expand to different types on our
currently supported nuttx targets. they are only signed integers under
riscv [^2].

[^1]: https://github.com/search?q=repo%3Aapache%2Fnuttx+%2F%5Cb_wchar_t%5Cb%2F&type=code
[^2]: https://c.godbolt.org/z/czMY79vsh
@rustbot

rustbot commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Comment thread src/unix/mod.rs
pub type uintptr_t = usize;
pub type ssize_t = isize;

#[cfg(not(target_os = "nuttx"))]

@no1wudi no1wudi Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not really, the latest commit replicates the nuttx definition 1 more closely. if anything, the link you provide matches the current definition (a c int and not necessarily a fixed-width integer.)

Footnotes

  1. https://github.com/apache/nuttx/blob/master/include/sys/types.h#L157

@tgross35 tgross35 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The current form LGTM but I'll wait for maintainer ack

View changes since this review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-maintainer S-waiting-on-review stable-nominated This PR should be considered for cherry-pick to libc's stable release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants