Skip to content

Conversation

@cosmicexplorer
Copy link

@cosmicexplorer cosmicexplorer commented Jul 3, 2025

Description

The method posix_getdents() has recently been added in POSIX issue 8 (from 2024). This method offers a standard interface to retrieve multiple directory entries at once into a preallocated buffer. This typically translates directly to a syscall using the existing SYS_getdents{,64} key.

However, as posix_getdents() is so new, it has not been universally implemented yet. musl added it immediately last year, but glibc does not yet have it. Instead, glibc >= 2.30 has the method getdents64(), which is Linux-only.

While musl appears to implement posix_getdents() for all supported platforms, it appears that the version of musl rust pulls in is not from the local machine, but from its own bundled copy. This copy does not have posix_getdents(), but instead simply has getdents64() to match glibc on linux, as well as a getdents() macro for compatibility with the BSD standard.

The extant BSDs just name their version of this method getdents(), without any 64 extension, and expose this as a libc method call instead of a syscall.

Surprisingly, macOS does not support any form of the getdents*() method. I mentioned this in passing to an Apple engineer a few weeks ago.

I have been in contact with a glibc developer who plans to support this, but cannot make any timeline guarantees. Since it appears that the BSDs also have yet to add support for the POSIX standardized posix_getdents() too, it does not seem terribly pressing to block this PR until upstream Rust pulls in a more recent version of musl. Instead, a followup PR should be able to triumphantly add posix_getdents() to all the POSIX platforms at once! ^_^ :D

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 O-gnu O-linux O-musl O-unix S-waiting-on-review stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Jul 3, 2025
@cosmicexplorer cosmicexplorer force-pushed the posix-getdents branch 2 times, most recently from d6e29f6 to 5d4300a Compare July 3, 2025 04:51
@cosmicexplorer
Copy link
Author

cosmicexplorer commented Jul 3, 2025

I do not understand why freebsd tests would be failing if I have not modified any code or semver tests for freebsd.

It appears the tests are failing on methods I have not modified (https://cirrus-ci.com/task/5573619788546048):

warning: libc-test@0.1.0: /tmp/cirrus-ci-build/target/x86_64-unknown-freebsd/debug/build/libc-test-ec9ac07491fdefaa/out/main.c:33518:59: error: invalid application of 'sizeof' to an incomplete type 'struct xktls_session_onedir'

@cosmicexplorer
Copy link
Author

cosmicexplorer commented Jul 3, 2025

Ok, now I'm getting a failure in a musl shard (aarch64-unknown-linux-musl) which I'm not sure about (https://github.com/rust-lang/libc/actions/runs/16041997001/job/45265389790?pr=4522):

warning: libc-test@0.1.0: /checkout/target/aarch64-unknown-linux-musl/debug/build/libc-test-c7eda42317ad320c/out/main.c: In function '__test_size_posix_dent':
warning: libc-test@0.1.0: /checkout/target/aarch64-unknown-linux-musl/debug/build/libc-test-c7eda42317ad320c/out/main.c:39376:56: error: invalid application of 'sizeof' to incomplete type 'struct posix_dent'
warning: libc-test@0.1.0: 39376 |  uint64_t __test_size_posix_dent(void) { return sizeof(struct posix_dent); }

The posix_dent struct is definitely defined in musl (https://github.com/bminor/musl/blob/86373b4999bfd9a9379bc4a3ca877b1c80a2a340/include/dirent.h#L21-L27). I did add posix_dent to libc-test/semver/linux-musl.txt -- I'm not sure if I'm supposed to do that or not for struct definitions. I was not able to get the local musl testing working on my machine due to a failure in build.rs for libc-test (see OP), so I'm having difficulty debugging this on my own.

Any pointers on how one would typically debug this would be appreciated. I know the header linux/can/j1939.h exists within /usr/include on my machine, but the build.rs is not picking it up, causing a failure to build main.c when running cargo test within the libc-test subdir.

@cosmicexplorer
Copy link
Author

Ok, after testing this locally against a separate project (since the libc-test crate build script continues to be broken and unable to find headers), I found that posix_getdents() was not found in the musl rust was linking against for some reason, even though my local musl definitely has posix_getdents(). I'm assuming rust is pulling in an earlier version of musl that's not the one on my machine, so I simply removed it for now. That means this only adds getdents64() on glibc for linux, and getdents{,64}() on musl for linux. I'll update the OP to match this.

@cosmicexplorer
Copy link
Author

Ok, it looks like the freebsd shards are failing on main too, so I'm going to assume that's not because of my changes.

@tgross35
Copy link
Contributor

Sorry this took a while to get to. Could you explain why this is needed? The manpage says:

This is not the function you are interested in. Look at readdir(3) for the POSIX conforming C library interface. This page documents the bare kernel system call interface.

Which sounds deprecated

@tgross35
Copy link
Contributor

For the followup needed,
@rustbot author

@rustbot
Copy link
Collaborator

rustbot commented Aug 27, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot
Copy link
Collaborator

rustbot commented Sep 13, 2025

Some changes occurred in OpenBSD module

cc @semarie

@cosmicexplorer cosmicexplorer force-pushed the posix-getdents branch 2 times, most recently from 3ffae61 to 0ea4693 Compare September 13, 2025 21:37
@rustbot
Copy link
Collaborator

rustbot commented Sep 13, 2025

Some changes occurred in solarish module

cc @jclulow, @pfmooney

Some changes occurred in the Android module

cc @maurer

Some changes occurred in OpenBSD module

cc @semarie

@rustbot rustbot added A-CI Area: CI-related items ctest Issues relating to the ctest crate O-android O-arm O-bsd O-linux-like O-macos labels Sep 13, 2025
@cosmicexplorer
Copy link
Author

I specifically described an inability to run local testing with cargo test --target mytarget above, and received no response, even though the checklist for every PR requires the user to run cargo test --target mytarget.

From reviewing recently merged PRs (https://github.com/rust-lang/libc/pulls?q=is%3Apr+is%3Aclosed), I can see that several changes were made to ctest, along with another libc release. Do these changes have any effect upon how to run tests locally? Is there anything I need to change in light of the changes that have been merged since I asked how to reproduce the tests locally as instructed?

I see that the most recent commit to main also fails on the same freebsd-13 shard: d2ece10. So it appears that the tests are not helpful at all, and my change is passing all of the CI that everything else is.

@tgross35 @semarie I have provided detailed links to manual pages. @tgross35, your first reply did not read the links I provided (according to the PR submission instructions) (along with a lurid degree of context), and instead you misinterpreted a link you searched on the web. There has since been no other responses. I see other changes such as #4729 with absolutely no information getting waved on through.

getdents() is a 2024 POSIX standard and I specifically need it in order to improve the performance of the rust stdlib fs::read_dir() method, greatly reducing the number of syscalls on supported systems. It will also drastically reduce the complexity of the code in the stdlib. This will help rust users.

Please help me understand how to progress this in order to improve the performance of the rust stdlib. If there is something missing here, I am very willing to do more work, but right now I don't know how to proceed and the only feedback I have received did not even read the links I provided. A 2024 POSIX standard is the opposite of deprecated and this method has been available in the same stable ABI across all these platforms for at least a decade.

I am also fine waiting, if it's necessary to block on CI fixes first. But this is blocking my further work on rustc and it's an exceptionally stable ABI across a wide variety of rust's supported platforms.

@rustbot ready

@semarie
Copy link
Contributor

semarie commented Sep 30, 2025

The getdents() addition is fine for the OpenBSD part.

Regarding how to make progress on the PR, I couldn't really help you. A friendly ping (like you did) is the way.
I agree that FreeBSD CI failure is unrelated with your PR.

Regarding #4729 you commented, the PR is part of a previous discussion I had with the OP (and I know him), I formally approved the PR as it touch only OpenBSD (and I agree that the PR description was very low and it is why I added some context in my comment). But I am not in position to merge it, so it will wait for someone else to merge it, as your current PR.

Regards.

@cosmicexplorer
Copy link
Author

cosmicexplorer commented Oct 1, 2025

Thanks so much for explaining that context. It is very reassuring to have this explanation and I appreciated your patience in describing how to understand these interactions. ^_^

@tgross35
Copy link
Contributor

Again, I'm sorry about the delay here. Note that reviews in this repo typically are in a 3-4 week cycle since I do them in batches, and currently I'm a couple weeks behind that so new API isn't the highest priority. There has been a pretty complete overhaul of testing and CI over the past few months, which likely explain some of what you've experienced.

There's a lot to address here:

While musl appears to implement posix_getdents() for all supported platforms, it appears that the version of musl rust pulls in is not from the local machine, but from its own bundled copy. This copy does not have posix_getdents(), but instead simply has getdents64() to match glibc on linux, as well as a getdents() macro for compatibility with the BSD standard.

For reference, Rust's musl gets bumped occasionally. rust-lang/rust#142682 may have what you are interested in.

I do not understand why freebsd tests would be failing if I have not modified any code or semver tests for freebsd.

FreeBSD was failing for a while at this time due to issues with the old ctest, as you later noted. It's a non-required job though. FreeBSD 13 has a spurious segfault in Cargo (not libc-specific).

I specifically described an inability to run local testing with cargo test --target mytarget above, and received no response, even though the checklist for every PR requires the user to run cargo test --target mytarget.

We do appreciate if anyone can test locally (especially for less common platforms), but there's no need to jump through hoops or bother with cross compiling.

cargo t -p libc-test is the usual one to check locally if you do. The occasional difference between local and CI is expected since some things change frequently. If the differences can be sanely explained then we defer to CI as being "correct" when this happens. For MS_NOUSER, see discussion in #4698. For the failure loading repos, you just happened to be running CI around the time of a ubuntu bump that required action.

From reviewing recently merged PRs (https://github.com/rust-lang/libc/pulls?q=is%3Apr+is%3Aclosed), I can see that several changes were made to ctest, along with another libc release. Do these changes have any effect upon how to run tests locally? Is there anything I need to change in light of the changes that have been merged since I asked how to reproduce the tests locally as instructed?

Were there other problems besides MS_NOUSER?

@tgross35 @semarie I have provided detailed links to manual pages. @tgross35, your first reply did not read the links I provided (according to the PR submission instructions) (along with a lurid degree of context), and instead you misinterpreted a link you searched on the web. There has since been no other responses. I see other changes such as #4729 with absolutely no information getting waved on through.

Note that this PR just contains a lot more information than most, and has some weird interactions (e.g. glibc's documentation is pretty unusual and worth rectifying).

getdents() is a 2024 POSIX standard and I specifically need it in order to improve the performance of the rust stdlib fs::read_dir() method, greatly reducing the number of syscalls on supported systems. It will also drastically reduce the complexity of the code in the stdlib. This will help rust users.

Rust's minimum glibc is 2.17 but getdents64 was added in 2.30 (per abilist), so std will still need to use the syscalls and isn't blocked on this for Linux.


Anyway, I just have a few small requests then this can merge. Please squash as well.

Comment on lines +977 to +976
extern "C" {
pub fn getdents(fd: c_int, buf: *mut crate::dirent, len: usize) -> c_int;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you combine this with the extern "C" block above?

Comment on lines +1347 to +1354
extern "C" {
// There is no glibc wrapper for the getdents system call, and getdents64() is only available
// from 2.30 onwards.
/// `buffer` points to a buffer of [`crate::dirent64`] structs.
pub fn getdents64(fd: c_int, buffer: *mut c_void, nbytes: usize) -> isize;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you merge this with the extern "C" block above?

Also no doc comments please, for this crate we defer to the platform manpages.

@@ -0,0 +1 @@
getdents64
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the file doesn't exist yet and Hurd is T3, it's not being checked. So this can just be dropped

Comment on lines +2960 to +2961

pub fn getdents(fd: c_int, buf: *mut c_char, nbytes: usize) -> c_int;
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment on lines +4821 to +4823
extern "C" {
pub fn getdents(fd: c_int, buf: *mut c_char, nbytes: usize) -> isize;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this move into an existing extern "C" block?

@tgross35 tgross35 changed the title add musl and glibc wrappers for getdents{,64} Add musl and glibc binings for getdents{,64} Oct 29, 2025
@tgross35 tgross35 changed the title Add musl and glibc binings for getdents{,64} Add musl and glibc bindings for getdents{,64} Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-CI Area: CI-related items ctest Issues relating to the ctest crate O-bsd O-linux O-musl S-waiting-on-author 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