glibc: Add f_type to statvfs and statvfs64 - #5434
Draft
valentynkit wants to merge 3 commits into
Draft
Conversation
Almost every glibc 32-bit arch declared an identical `statvfs64`. Six of the eight were fully the same, so they move to the shared `b32/mod.rs`. `m68k` and `riscv32` differ, so they are excluded by cfg, and keep their own declarations. No functional change: shared declaration is byte-for-byte identical to the six it replaces.
No functional changes: shared declaration is byte for byte identical to architecture specifics it replaces, only `riscv64` differs, and guarded by cfg, to use it own declaration.
glibc 2.39 allocated one of the six spares to `f_type` of type unsigned int. Link: https://github.com/sailfishos-mirror/glibc/blob/ef321e23c20eebc6d6fb4044425c00e6df27b05f/sysdeps/unix/sysv/linux/bits/statvfs.h#L75-L76 Link: https://github.com/sailfishos-mirror/glibc/blob/ef321e23c20eebc6d6fb4044425c00e6df27b05f/sysdeps/unix/sysv/linux/bits/statvfs.h#L54-L55
rustbot
added
O-arm
O-gnu
O-linux
O-linux-like
O-mips
O-powerpc
O-riscv
O-sparc
O-unix
O-x86
stable-nominated
Contributor
Author
|
Is it okay to have it as one PR, or should dedup in gnu/b32 and gnu/b64 be separate PR? |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
glibc 2.39 allocated one of the six reserved
__f_spareints tof_typeforstatvfsandstatvfs64, which mirrorsstatfs.sailfishos-mirror/glibc@92861d9
Adding
f_typetostatvfsandstatvfs64for linux/gnu, usingc_uint.Also deduplicated
statvfs64for 32-bit arch, it was declared byte-for-byte identical on six of the eight platforms, so it was moved to the sharedb32/mod.rs(m68kandriscv32differ, so they are excluded by cfg, and keep their own declaration). And deduplicatedstatvfsandstatvfs64for 64-bit arch, keeping onlyriscv64excluded by cfg, and keeping its own declaration.There is also musl that is missing
f_type. It wasn't addressed in this PR, should I address musl also, or any other not just glibc?Also
riscv32andriscv64declare their__f_sparefield as pub, it feels inconsistent with the other declarations, and forriscv64it is the reason it keeps its own declaration (riscv32differs in also lacking__f_unused). Is there a specific reason to have itpub __f_spare, or should this be addressed and fixed?Closes #4121
Checklist
libc-test/semverhave been updated, no change needed.cargo test -p libc-test --target mytarget); especially relevant for platforms that may not be checked in CI@rustbot label +stable-nominated