nuttx: clean up module#5245
Conversation
Add types that were missing from our bindings to the NuttX interface [^1]. [^1]: https://github.com/apache/nuttx/blob/master/include/sys/types.h
Fix `ino_t` definition to fit that of [^1]. [^1]: https://github.com/apache/nuttx/blob/master/include/sys/types.h#L163
Use `gnu_file_offset_bits64` `cfg` to export LFS bindings. NuttX follows that "64"-suffixed symbols are exposed only if large file support is enabled [^1] [^2]. We choose not to expose those because it's redundant. Otherwise, the existing `cfg` serves the same purpose as upstream's `CONFIG_FS_LARGEFILE` build option. [^1]: https://github.com/apache/nuttx/blob/master/include/sys/types.h#L88-L95 [^2]: https://github.com/apache/nuttx/blob/master/include/sys/types.h#L199-L226
Add `cfg` to use types for the small memory model in NuttX. This can force use of narrower pointer-sized types (i.e. `size_t` and `ssize_t`.) This is a build-time option upstream [^1] [^2]. [^1]: https://github.com/apache/nuttx/blob/master/include/sys/types.h#L120-L132 [^2]: https://github.com/search?q=repo%3Aapache%2Fnuttx+path%3Aarch%2F**%2F*+%2F%5Cb_ssize_t%5Cb%2F&type=code
rustbot
added
S-waiting-on-review
stable-nominated
| }; | ||
|
|
||
| pub type id_t = c_int; | ||
| pub type key_t = u32; |
There was a problem hiding this comment.
"nuttx: add missing types": Isn't this signed? https://github.com/apache/nuttx/blob/9e467388de0285c1c160681aeafaa9c6b5723ac0/include/sys/types.h#L170
Make sure your links to source are permalinks (available under the triple dots) so we know what things looked like if they change in the future.
| pub type dev_t = i32; | ||
| pub type fsblkcnt_t = u64; | ||
| pub type loff_t = off_t; | ||
| pub type cpu_set_t = u32; |
There was a problem hiding this comment.
"nuttx: add missing types": Is this definition used? I'd prefer to omit it because of the volatile
| pub type socklen_t = u32; | ||
| pub type speed_t = usize; | ||
| pub type suseconds_t = i32; | ||
| pub type useconds_t = i32; |
There was a problem hiding this comment.
| pub type tcflag_t = u32; | ||
| pub type clockid_t = i32; | ||
| pub type time_t = i64; | ||
| pub type timer_t = *mut c_void; |
There was a problem hiding this comment.
Eek far pointers in the definition. Is this actually needed?
| pub type key_t = u32; | ||
| pub type nlink_t = u16; | ||
| pub type ino_t = u16; | ||
| pub type ino_t = u32; |
There was a problem hiding this comment.
"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.
| pub type wchar_t = i32; | ||
| pub type wchar_t = c_int; |
There was a problem hiding this comment.
"nuttx: mirror type definitions" Seems fine but where does this one come from?
| pub type locale_t = *mut i8; | ||
| pub type mode_t = u32; | ||
| pub type nfds_t = u32; | ||
| pub type off_t = i64; |
There was a problem hiding this comment.
"nuttx: provide correct lfs support": It looks like we were already supporting CONFIG_FS_LARGEFILE - why the change?
| } | ||
|
|
||
| cfg_if! { | ||
| if #[cfg(nuttx_small_mm)] { |
There was a problem hiding this comment.
"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.
3 participants
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_MEMORYandCONFIG_FS_LARGEFILE. The former enables types with pointer bit width (size_tandssize_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
libc-test/semverhave been updated*LASTor*MAXare included (see #3131)cd libc-test && cargo test --target mytarget); especially relevant for platforms that may not be checked in CI@rustbot label +stable-nominated