Skip to content

Handle getSize in File for windows pipes#24873

Open
marler8997 wants to merge 1 commit into
ziglang:masterfrom
marler8997:windowsGetPipeSize
Open

Handle getSize in File for windows pipes#24873
marler8997 wants to merge 1 commit into
ziglang:masterfrom
marler8997:windowsGetPipeSize

Conversation

@marler8997

Copy link
Copy Markdown
Contributor

Fixes #24867

Comment thread lib/std/fs/File.zig Outdated
Comment thread lib/std/fs/File.zig Outdated
@marler8997

Copy link
Copy Markdown
Contributor Author

Thanks @squeek502 and @The-King-of-Toasters for the pointers...I've updated to use NtQueryVolumeInformationFile + FileFsDeviceInformation

@andrewrk

Copy link
Copy Markdown
Member

Really appreciate the sleuthing help on this one, everyone. Based on this information I updated #24865 to also pre-set size_err when initializing in streaming mode. I think that's a nice optimization regardless, and it avoids these two issues enough to make them non release blockers. So at this point, what we're left with is fixing the case where a file is initialized in the default mode, it needs to detect properly when it should be downgraded to streaming mode.

Comment thread lib/std/fs/File.zig Outdated
@The-King-of-Toasters

Copy link
Copy Markdown
Contributor

It seems similar logic is in File.isCygwinPty. Maybe worth splitting it out into its own function?

@squeek502

Copy link
Copy Markdown
Member

I think the similarities are mostly superficial--it's basically just that they both call NtQueryVolumeInformationFile with FILE_FS_DEVICE_INFORMATION, so the potential consolidation would just be the addition of a thin wrapper around that particular combination which doesn't feel super necessary.

(I'm not opposed to it, though, that's just my initial reaction)

@marler8997

Copy link
Copy Markdown
Contributor Author

I'm vibing with your "initial reaction" squeek. Creating a thin wrapper function in this case feels it could be an "abstraction trap"...resist the urge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

standalone_test_cases.test_obj_link_run failing on windows

5 participants