minimal dirfd implementation (1/4) #146341

Qelxiros · 2025-09-08T17:32:07Z

This is the first of four smaller PRs that will eventually be equivalent to #139514.

A few notes:

I renamed new to open because open_dir takes &self and opens a subdirectory.
- I also renamed open to open_file.
I'm not sure how to impl AsRawFd and friends because the common implementation uses PathBufs. How should I proceed here?

The other PRs will be based on this one, so I'll make drafts and mark them ready as their predecessors get merged. They might take a bit though; I've never done this particular thing with git before.

Tracking issue: #120426

r? @tgross35

try-job: aarch64-apple
try-job: dist-various*
try-job: test-various*
try-job: x86_64-msvc-1
try-job: x86_64-mingw*

library/std/src/sys/fs/common.rs

library/std/src/sys/fs/uefi.rs

library/std/src/sys/fs/unsupported.rs

library/std/src/fs.rs

library/std/src/sys/fs/windows.rs

tgross35 · 2025-09-10T03:01:20Z

@ChrisDenton I'll need your help for the Windows review

library/std/src/sys/fs/unix.rs

Qelxiros · 2025-09-10T17:19:09Z

@rustbot ready

tgross35

Looks pretty good to me, just a few mechanical things here. There are a couple left over from the previous review, #146341 (comment), #146341 (comment), and (newly) #146341 (comment).

View changes since this review

library/std/src/sys/fs/unix/dir.rs

library/std/src/sys/fs/unsupported.rs

library/std/src/sys/fs/unix.rs

library/std/src/sys/fs/windows.rs

library/std/src/sys/fs/unix.rs

library/std/src/sys/fs/windows/dir.rs

tgross35 · 2025-09-10T21:19:14Z

library/std/src/sys/fs/windows/dir.rs

+ let mut handle = ptr::null_mut();
+ let mut io_status = c::IO_STATUS_BLOCK::PENDING;
+ let access = opts.get_access_mode()? | c::SYNCHRONIZE;
+ let options = create_options | c::FILE_SYNCHRONOUS_IO_NONALERT;


Could you add a note about why this flag is set?

I no longer remember why I chose this one, and looking at it now, I'm not sure whether we should set this one, FILE_SYNCHRONOUS_IO_ALERT, or neither. Maybe @ChrisDenton has thoughts?

Update, using neither causes an error, so I've added back FILE_SYNCHRONOUS_IO_NONALERT

Yeah, passing this flag is equivalent to not passing FILE_FLAG_OVERLAPPED to CreateFile. So if you don't pass this flag, other APIs using the handle need to follow the usual overlapped rules, otherwise they may behave improperly.

tgross35 · 2025-09-10T21:24:27Z

From the top post:

* I'm not sure how to `impl AsRawFd` and friends because the `common` implementation uses `PathBuf`s. How should I proceed here?

I think it would be fine to include the impl AsRawFd only in unix/dir so it's only implemented if we have dirfd. I'll add an unresolved question to the tracking issue.

The other PRs will be based on this one, so I'll make drafts and mark them ready as their predecessors get merged. They might take a bit though; I've never done this particular thing with git before.

That is quite alright, there is no hurry :) For reference, the --update-refs rebase flag can be pretty useful when you're working with a stack (e.g. https://andrewlock.net/working-with-stacked-branches-in-git-is-easier-with-update-refs/)

minimal dirfd implementation (1/4) try-job: aarch64-apple try-job: dist-various* try-job: test-various* try-job: x86_64-msvc* try-job: x86_64-mingw

minimal dirfd implementation (1/4) try-job: aarch64-apple try-job: dist-various* try-job: test-various* try-job: x86_64-msvc* try-job: x86_64-mingw*

tgross35 · 2025-10-20T17:56:45Z

For the suggestion above and a rebase,

@rustbot author

rustbot · 2025-10-20T17:56:49Z

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

rustbot · 2025-10-20T18:27:10Z

This PR was rebased onto a different master 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.

Qelxiros · 2025-10-20T18:28:38Z

@rustbot ready

tgross35 · 2025-10-20T19:00:21Z

@bors2 try

minimal dirfd implementation (1/4) try-job: aarch64-apple try-job: dist-various* try-job: test-various* try-job: x86_64-msvc-1 try-job: x86_64-mingw*

rust-bors · 2025-10-20T21:05:38Z

💔 Test for 7d804a8 failed: CI. Failed jobs:

try - x86_64-msvc-1 (web logs, extended logs)

tgross35 · 2025-10-27T20:14:15Z

I'm not quite sure what's failing here unfortunately. @neerajsi-msft do you have any ideas? The failure was in the test_dir_read_file test at

dir.open_file("foo.txt") failed with: The filename, directory name, or volume label syntax is incorrect. (os error 123)

@rustbot author

neerajsi-msft · 2025-10-29T23:02:53Z

I'm not quite sure what's failing here unfortunately. @neerajsi-msft do you have any ideas? The failure was in the test_dir_read_file test at
dir.open_file("foo.txt") failed with: The filename, directory name, or volume label syntax is incorrect. (os error 123) 
@rustbot author

I'll try to repro locally and take a look.

ChrisDenton · 2025-10-30T15:16:49Z

library/std/src/sys/fs/windows/dir.rs

+ if path.is_absolute() {
+ return File::open(path, opts);
+ }
+ let path = to_u16s(path)?;


to_u16s produces a null-terminated path whereas kernel paths treat nulls literally rather than as a terminator (much like rust strings). I believe this is what's causing the test to fail.

Yes, I debugged this locally and Chris Denton is right. The NULL terminator is causing a problem.

Qelxiros · 2025-10-30T15:36:05Z

Looks like I need a try job to see if the error still exists.
@bors2 try

rust-bors · 2025-10-30T15:36:08Z

@Qelxiros: 🔑 Insufficient privileges: not in try users

ChrisDenton · 2025-10-30T18:05:08Z

@bors2 try

minimal dirfd implementation (1/4) try-job: aarch64-apple try-job: dist-various* try-job: test-various* try-job: x86_64-msvc-1 try-job: x86_64-mingw*

rust-bors · 2025-10-30T21:10:53Z

☀️ Try build successful (CI)
Build commit: b34b92d (b34b92dc58537d82f2e3477bddd7c9560f59aae5, parent: 8205e6b75ec656305ac235d4726d2c7a1ddcef14)

rustbot assigned tgross35 Sep 8, 2025

rustbot added O-windows Operating system: Windows S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 8, 2025

Qelxiros force-pushed the dirfd-minimum branch from c3278f6 to 6982a46 Compare September 8, 2025 17:37

This comment has been minimized.

Sign in to view

Qelxiros force-pushed the dirfd-minimum branch from 6982a46 to 313c731 Compare September 8, 2025 18:26

This comment has been minimized.

Sign in to view

Qelxiros force-pushed the dirfd-minimum branch from 313c731 to 1b3d7d6 Compare September 8, 2025 18:55

This comment has been minimized.

Sign in to view

Qelxiros force-pushed the dirfd-minimum branch from 1b3d7d6 to be0d761 Compare September 9, 2025 15:09

tgross35 requested changes Sep 10, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 10, 2025

tgross35 requested a review from ChrisDenton September 10, 2025 03:01

Qelxiros force-pushed the dirfd-minimum branch from be0d761 to 3083d58 Compare September 10, 2025 17:17

Qelxiros commented Sep 10, 2025

View reviewed changes

library/std/src/sys/fs/unix.rs Outdated Show resolved Hide resolved

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 10, 2025

tgross35 reviewed Sep 10, 2025

View reviewed changes

This comment was marked as outdated.

Sign in to view

rust-bors bot added a commit that referenced this pull request Sep 10, 2025

Auto merge of #146341 - Qelxiros:dirfd-minimum, r=<try>

0ac54d6

minimal dirfd implementation (1/4) try-job: aarch64-apple try-job: dist-various* try-job: test-various* try-job: x86_64-msvc* try-job: x86_64-mingw

This comment has been minimized.

Sign in to view

This comment was marked as outdated.

Sign in to view

rust-bors bot added a commit that referenced this pull request Sep 10, 2025

Auto merge of #146341 - Qelxiros:dirfd-minimum, r=<try>

ba649e4

minimal dirfd implementation (1/4) try-job: aarch64-apple try-job: dist-various* try-job: test-various* try-job: x86_64-msvc* try-job: x86_64-mingw*

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 20, 2025

Qelxiros force-pushed the dirfd-minimum branch from 18a93b3 to 70ea0df Compare October 20, 2025 18:19

This comment has been minimized.

Sign in to view

Qelxiros force-pushed the dirfd-minimum branch from 70ea0df to fedd376 Compare October 20, 2025 18:27

Qelxiros force-pushed the dirfd-minimum branch from fedd376 to 1624481 Compare October 20, 2025 18:28

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 20, 2025

rust-bors bot added a commit that referenced this pull request Oct 20, 2025

Auto merge of #146341 - Qelxiros:dirfd-minimum, r=<try>

7d804a8

minimal dirfd implementation (1/4) try-job: aarch64-apple try-job: dist-various* try-job: test-various* try-job: x86_64-msvc-1 try-job: x86_64-mingw*

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 27, 2025

ChrisDenton reviewed Oct 30, 2025

View reviewed changes

Add Dir::open(_file) and some trait impls

1479928

Qelxiros force-pushed the dirfd-minimum branch from 1624481 to 1479928 Compare October 30, 2025 15:34

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Oct 30, 2025

Auto merge of #146341 - Qelxiros:dirfd-minimum, r=<try>

b34b92d

minimal dirfd implementation (1/4) try-job: aarch64-apple try-job: dist-various* try-job: test-various* try-job: x86_64-msvc-1 try-job: x86_64-mingw*

minimal dirfd implementation (1/4) #146341

Are you sure you want to change the base?

minimal dirfd implementation (1/4) #146341

Uh oh!

Conversation

Qelxiros commented Sep 8, 2025 • edited by tgross35 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tgross35 commented Sep 10, 2025

Uh oh!

Qelxiros commented Sep 10, 2025

tgross35 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tgross35 Sep 10, 2025

Choose a reason for hiding this comment

Qelxiros Sep 11, 2025

Choose a reason for hiding this comment

Qelxiros Sep 15, 2025

Choose a reason for hiding this comment

neerajsi-msft Oct 20, 2025

Choose a reason for hiding this comment

tgross35 commented Sep 10, 2025

This comment was marked as outdated.

This comment has been minimized.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

tgross35 commented Oct 20, 2025

rustbot commented Oct 20, 2025

This comment has been minimized.

rustbot commented Oct 20, 2025

Qelxiros commented Oct 20, 2025

tgross35 commented Oct 20, 2025

This comment has been minimized.

This comment has been minimized.

rust-bors bot commented Oct 20, 2025

tgross35 commented Oct 27, 2025

neerajsi-msft commented Oct 29, 2025

ChrisDenton Oct 30, 2025

Choose a reason for hiding this comment

neerajsi-msft Oct 30, 2025

Choose a reason for hiding this comment

Qelxiros commented Oct 30, 2025

rust-bors bot commented Oct 30, 2025

ChrisDenton commented Oct 30, 2025

This comment has been minimized.

rust-bors bot commented Oct 30, 2025

Labels

8 participants

Qelxiros commented Sep 8, 2025 •

edited by tgross35

Loading

tgross35 left a comment •

edited

Loading