Repository navigation
fix(libc): open a directory with fopen in a read mode on Windows - #8550
Conversation
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
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.
|
|
@Retr0G28 Thanks. The FreeBSD references and the descriptor-reuse check make this easy to follow, and the registration from libkernel follows the existing One request, about what a caller sees after the Could the read functions on a directory stream fail the way the Linux host does instead of throwing: return 0 or EOF, set the stream's error flag, and set After that, the bar is a full CI run with |
c0a66be to
40764df
Compare
|
@oneandonlydean Thanks, agreed: a stop where the caller had an error it could handle is the wrong trade, so the throws are gone. Rebased on A directory stream now behaves the way it does on a Linux host:
Most of this comes from the host for free. I measured a The |
40764df to
f6e4c06
Compare
FreeBSD 11 fopen opens the path with _open, and vn_open_vnode refuses a directory only for FWRITE or O_TRUNC, so fopen of a directory in a read mode succeeds; Linux hosts already open it. On Windows CreateFileW without FILE_FLAG_BACKUP_SEMANTICS refused it with EACCES. fopen now opens such a path as a libkernel directory descriptor, which libkernel registers in libc when it loads, and fclose and freopen remove it from libkernel's table. Reads on the stream fail with EISDIR and set the error indicator, as on a Linux host and FreeBSD's tmpfs: the Windows CRT fails them already but reports EINVAL. Writes fail with EBADF, and fseek, ftell and ungetc work, as on any read-only stream.
f6e4c06 to
3622674
Compare
|
Independent Windows run of this PR (head 3622674). Environment: Windows 11 (10.0.26300), Ryzen 7 9800X3D, WinLibs GCC 15.2.0 r7 (MinGW), Release, BUILD_TESTING=ON. I built only the libs and the five test targets below. Results:
Not run: Linux/macOS, the rest of the 60-test selection in the description, the full repository CTest, and any title. AI-assisted: yes |
|
@Retr0G28 thanks, this is what I hoped for: reading a directory stream now fails with EISDIR and sets the error indicator instead of throwing, and the new cases cover fread, fgetc, fgets and fscanf. Sorry for the slow reply. I'm sending it for a full CI run. |
What
On Windows,
fopenof a directory in a read mode (r,rb,re) failed withEACCES, becauseCreateFileWwithoutFILE_FLAG_BACKUP_SEMANTICSrefuses a directory. FreeBSD 11fopenopens the path with_open(lib/libc/stdio/fopen.c), andvn_open_vnoderefuses a directory only forFWRITEorO_TRUNC(sys/kern/vfs_vnops.c), so the call succeeds there. Linux hosts already open it. #8192 already made the write modes fail withEISDIR.When the read-mode open fails with
EACCESand the path is a directory,fopennow opens it as a libkernel directory descriptor (File::OpenDirectoryDescriptor), the same kind of descriptoropenreturns for a directory, and wraps it in the stream. libc cannot link libkernel, since libkernel links libc, so libkernel registers its open and forget functions in libc when it loads (LibcRegisterDirectoryStreams_nid_no_patch).fcloseandfreopenremove the descriptor from libkernel's table before closing it, so a later file that gets the same number is not taken for a directory.The stream then behaves as a directory stream does on a Linux host:
fread,fgetc,getc_unlocked,fgets,fscanf,vfscanf, and the wide andgetcforms built on them) returns 0,EOForNULL, sets the error indicator and setserrnotoEISDIR. The Windows CRT already fails the read and sets the indicator (ReadFileon a directory handle fails withERROR_INVALID_FUNCTION), but reportsEINVAL, or leaveserrnoalone infscanf, so a failed read on a directory stream with the indicator set reportsEISDIR;EBADFand sets the error indicator, as on any read-only stream;fseek,ftell,ungetc,fileno,feof,ferror,clearerr,setvbufand the lock functions work, andfflushreturns 0, as FreeBSD does on a stream that is not open for writing.FreeBSD 11's own result for a read depends on the filesystem:
tmpfs_readfails withEISDIR(sys/fs/tmpfs/tmpfs_vnops.c), whileffs_readreturns the raw directory blocks (sys/ufs/ffs/ffs_vnops.c). What the console's filesystems return is unverified, which the entry indocs/dev/debt/libc.mdrecords. That entry is also cut to the part that is still open:freopenonto a directory in a read mode still fails withEACCES.Tests:
guest_filesystemopens a directory withr,rbandreon every host. For each mode it checks:filenoplussceKernelFstatreport a directory;feof,ferrorandfflushreturn 0;fread,fgetc,fgetsandfscanffail withEISDIRand set the error indicator, whichclearerrclears;fseekandftellsucceed at 0;fputcfails withEBADF.On Windows it then closes the stream and checks that
openof a regular file reuses the descriptor number and reads the file's data.file_streamsexpected the old WindowsEACCESforrb. It now checkswbandEISDIRon every host. That test links only libc, so no directory streams are registered in it.Tested
Windows 11, WinLibs MinGW-w64 GCC 15.2.0 r7, CMake + Ninja, Release,
-DBUILD_TESTING=ON, rebased onmain47fa6ee.main's libc,guest_filesystemfails at thefopencheck:Filesystem check failed at line 609(guest errno 13,EACCES).guest_filesystempasses. Iffclosedoes not remove the descriptor from libkernel's table,guest_filesystemfails at the reuse check:readon the new file returns guest errno 21.fseek/ftell/ungetcsucceeding) were measured with a standalone program on a_fdopenstream over a directory handle.-k 0: the only failure is the MinGW alignment error inImageArrayWindowPastLayerLimit.cppthat is already onmain(fixed by test(agc): align the array window test's texels at run time #8310). ctest-R "fopen|stdio|stream|file|directory|libc|scanf|printf|random|errors|dup|fcntl" -E "^agc_|guest_host_libc|windows_working_directory": 60 of 60 pass (the two excluded tests need the relinker binary); a test that timed out under parallel load passes when run alone.python tools/check_conventions.py --base origin/mainpasses.guest_filesystemblock now runs there too. It expects glibc's behaviour on a directory stream (EISDIRfromread,EBADFfrom a write to a read-only stream,lseekon a directory succeeding), so the Linux CI run is the check.Overlap (
tools/pr_overlap.py):DirectoryDescriptor.cpp, and feat(libc): support archive-backed stdio reads #7447 editsFileStream::Closenext to the new lines. Whichever merges second needs a small rebase. Neither opens directory streams.SwitchToReadingbefore each read; the directory reads keep going through it, since a directory stream is not an update stream. fix(libc): support character reads from random device streams #8150 changes the same stdio files and does not touch directory streams.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)