Skip to content

fix(libkernel): getdirentries reports the file number lstat reports on Windows - #11311

Closed
Gruzzberry wants to merge 1 commit into
boykopovar:mainfrom
Gruzzberry:fix/getdirentries-file-number
Closed

Gruzzberry wants to merge 1 commit into
boykopovar:mainfrom
Gruzzberry:fix/getdirentries-file-number

Conversation

@Gruzzberry

@Gruzzberry Gruzzberry commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

What

On Windows getdirentries and getdents numbered the entries of a directory 1, 2, 3 in the order they were read, so d_fileno never matched the st_ino that stat and lstat report for the same file, and the same name could get a different number after the directory changed and was read again. FreeBSD 11.4's ufs_readdir (ufs_vnops.c) sets dstdp.d_fileno = dp->d_ino, the inode stat returns. The libc readdir already does this on Windows, taking the file id from the directory listing (FileIdBothDirectoryInfo) through WindowsFileNumber_nid_no_patch; the kernel call now reads the same listing and uses the same function, so both give the same number for every entry, and for entries lstat can read (the common case, accented and Japanese names and links included) that is also the number lstat reports. Two cases are not covered: a file system that cannot list file ids (libc falls back to a hash of the name, the kernel call keeps the running number it had before), and .. at the guest root and . or .. at a volume root, which are host directories.

The same probe found nothing else to change in getdirentries that is not already covered:

  • The record length is (8 + namlen + 1 + 3) & ~3, the 4-byte GENERIC_DIRSIZ of FreeBSD 11, and names in UTF-8 are reported byte for byte.
  • A buffer too small for the first entry gives EINVAL, a descriptor that is not a directory EINVAL, a closed one EBADF, a null buffer EFAULT, and the end of the directory 0.
  • d_type for Windows links is the subject of fix(libkernel): classify Windows directory links by app0 scope #8726 and is left alone. A zero-length buffer returns EINVAL here and an existing test pins that; kern_getdirentries in FreeBSD would return 0, which I did not change.

Tested

Windows 11 (10.0.26200), WinLibs MinGW-w64 GCC 15.2.0 r7, Ninja, Release, -DBUILD_TESTING=ON, on current main.

  • guest_directory_entries: new checks (Windows only) that d_fileno of every entry read with getdents equals the st_ino of lstat on the same name, for the fixture directory and for a directory with a file link and file names containing an accented letter and Japanese characters, and that the numbers of existing names do not change after another file is added and the directory is read again. On main the test fails; with this change it passes, three runs out of three.
  • Full suite: the only failures are pr_overlap_encoding, agc_host_import_cut_lifetime and agc_driver_depth_plane_partial_store, which fail on main in this environment too.
  • python tools/check_conventions.py --base upstream/main: passes.
  • Not tested: Linux and macOS (the change is Windows-only), a console, a title run.

Overlaps

#8520, #8726 and #9947 change nearby lines of DirectoryDescriptor.cpp: proposed on each that it goes first and this PR rebases.

Checklist

AI-assisted (Claude Code).

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 9947 core/libs/prx/libkernel/File/src/DirectoryDescriptor.cpp
PR 10552 core/libs/prx/libkernel/File/src/DirectoryDescriptor.cpp

@Gruzzberry
Gruzzberry force-pushed the fix/getdirentries-file-number branch 2 times, most recently from 160c10b to 3997396 Compare October 26, 2026 03:17
@Gruzzberry
Gruzzberry marked this pull request as ready for review October 26, 2026 03:17

@MarcZX10 MarcZX10 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 NTFS/file-ID path is well targeted: it reuses WindowsFileNumber_nid_no_patch and the Windows regression compares d_fileno with lstat across stable rereads, Unicode names and a symlink. I found one explicitly acknowledged uncovered behavior that needs resolution before approval: when GetFileInformationByHandleEx cannot supply file IDs, libc falls back to StableFileNumber(seed, fileName) in Directory.cpp, but this kernel path assigns enumeration-order numbers. In that case getdents still disagrees with readdir/lstat, and insertion order can change a name’s value. Please either use the same fallback algorithm here or record this known gap in docs/dev/debt/libkernel.md as required for known behavior gaps. The volume-root ./.. exception should be included in the note if it remains. Hosted conventions, eligibility, intake, labels and report pass; Windows product jobs were skipped by path policy, so the author-reported WinLibs test is the available platform execution evidence.

@github-actions github-actions Bot added libkernel needs rebase Conflicts with main, rebase needed labels Oct 26, 2026
…n Windows

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@Gruzzberry
Gruzzberry force-pushed the fix/getdirentries-file-number branch from 3997396 to ed7b7e9 Compare October 26, 2026 09:30
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 26, 2026
@adriadam10 adriadam10 self-assigned this Oct 27, 2026
@adriadam10

Copy link
Copy Markdown
Collaborator

Closing as superseded: since #8520 (0a64d23) Load in DirectoryDescriptor.cpp fills every entry, . and .. included, from File::FillLinkStat(...).st_ino, the same function lstat_nid_postfix uses (Stdio.cpp:1138). getdirentries no longer numbers entries by read order, so d_fileno already matches lstat on Windows, and the file-id fallback concern raised in review no longer applies (there is no separate listing any more).

This PR conflicts with main in seven hunks of that file and almost none of its production code would survive the rebase.

The test is still useful: GuestDirectoryEntries on main does not compare d_fileno with st_ino. You are welcome to open a test-only PR with the Windows checks (d_fileno == lstat st_ino for plain, accented, Japanese and link names, and stable numbers after adding a file), verified against current main.

@adriadam10 adriadam10 closed this Oct 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libkernel needs rebase Conflicts with main, rebase needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants