Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GetFileInformationByHandle compile error #14829

Merged
merged 4 commits into from
Mar 19, 2023

Conversation

rdunnington
Copy link
Contributor

The wrapper function was mistakenly referencing ntdll.zig when the actual function is declared in kernel32.zig.

The wrapper function was mistakenly referencing ntdll.zig when the actual function is declared in kernel32.zig.
@matu3ba
Copy link
Contributor

matu3ba commented Mar 7, 2023

LGTM. Eventually we'd like to use the Ntdll calls for file stuff, because this is highly unlikely to change. For file information this would be NtQueryInformationFile. see #1840.

@squeek502
Copy link
Collaborator

squeek502 commented Mar 7, 2023

Eventually we'd like to use the Ntdll calls for file stuff

This is already the case, unless you mean that Zig should supply a GetFileInformationByHandle implementation using NtQueryInformationFile.

std.os.windows.GetFileInformationByHandle is entirely unused in the std library (hence why this compile error was not found until now) and the only other related reference to GetFileInformationByHandle is one usage of std.os.windows.kernel32.GetFileInformationByHandleEx here.

@rdunnington
Copy link
Contributor Author

Ok cool, is there anything else I can do to get this merged? Or just wait until an admin approves the PR? It's my first contribution so I'm not sure how this works.

@squeek502
Copy link
Collaborator

Nothing you need to do, just need to wait.

@andrewrk
Copy link
Member

andrewrk commented Mar 8, 2023

Thank you for the patch.

I think I would prefer to entirely remove this function from the standard library rather than fix it.

Related issue: #4426

@rdunnington
Copy link
Contributor Author

Ok, that makes sense given the std lib doesn't use this function at all.

@rdunnington
Copy link
Contributor Author

rdunnington commented Mar 9, 2023

Latest change delete the function so no one will make the same mistake again. :) I think #14841 should probably clean up the dangling GetFileInformationByHandleEx declaration in kernel32.zig as well.

@kubkon kubkon enabled auto-merge (squash) March 19, 2023 16:36
@kubkon kubkon merged commit 30427ff into ziglang:master Mar 19, 2023
@rdunnington rdunnington deleted the windows-api-fix branch March 20, 2023 01:49
truemedian pushed a commit to truemedian/zig that referenced this pull request Mar 30, 2023
* Fix GetFileInformationByHandle compile error

The wrapper function was mistakenly referencing ntdll.zig when the actual function is declared in kernel32.zig.

* delete GetFileInformationByHandle since it's not used by the stdlib
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.

5 participants