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

Windows: Rework kernel32 apis #19641

Merged
merged 2 commits into from
Jul 18, 2024

Conversation

The-King-of-Toasters
Copy link
Contributor

@The-King-of-Toasters The-King-of-Toasters commented Apr 13, 2024

To facilitate #1840, this commit slims std.windows.kernel32 to only have the functions needed by the standard library. Since this will break projects that relied on these, I offer these solutions:

  • Make an argument as to why certain functions should be added back in. Note that they may just be wrappers around ntdll APIs, which would go against Prefer depending on NtDll rather than kernel32 or other higher level DLLs #1840.

    If necessary I'll add them back in and make wrappers in std.windows for it.

  • Maintain your own list of APIs. This is the option taken by bun, where they wrap functions with tracing.

  • Use zigwin32.

I've also added TODO comments that specify which functions can be reimplemented using ntdll APIs in the future.

Other changes:

  • Group functions into groups (I/O, process management etc.).
  • Synchronize definitions against Microsoft documentation to use the proper parameter types/names.
  • Break all functions with parameters over multiple lines.

@The-King-of-Toasters The-King-of-Toasters changed the title QueryObjectName: Add error union Windows: Rework kernel32 apis Apr 13, 2024
@The-King-of-Toasters
Copy link
Contributor Author

I've added fleshed out the comments on the remaining APIs. I've categorised them into three groups:

  • Forwarders: The function puts the arguments into place and jumps directly into the ntdll function.
  • Wrappers: The function sets up local variables and calls the ntdll function. Typically OBJECT_ATTRIBUTES or UNICODE_STRING, but can be more involved.
  • Unnamed: Functions that are too hard to simply replace.

@The-King-of-Toasters The-King-of-Toasters force-pushed the windows-api-refactor branch 2 times, most recently from 3e99b86 to ab020f0 Compare April 20, 2024 03:38
lib/std/os/windows/kernel32.zig Outdated Show resolved Hide resolved
lib/std/os/windows/kernel32.zig Outdated Show resolved Hide resolved
lib/std/os/windows/kernel32.zig Outdated Show resolved Hide resolved
@The-King-of-Toasters The-King-of-Toasters force-pushed the windows-api-refactor branch 2 times, most recently from 60252bb to 7615944 Compare April 22, 2024 07:12
lib/std/os/windows.zig Outdated Show resolved Hide resolved
@The-King-of-Toasters The-King-of-Toasters force-pushed the windows-api-refactor branch 2 times, most recently from 57f8ff0 to 6fb865e Compare April 25, 2024 07:15
@The-King-of-Toasters The-King-of-Toasters force-pushed the windows-api-refactor branch 2 times, most recently from 3999adb to 98b3e14 Compare May 12, 2024 14:25
@The-King-of-Toasters The-King-of-Toasters force-pushed the windows-api-refactor branch 3 times, most recently from aa26b64 to 44b364c Compare May 29, 2024 04:07
To facilitate ziglang#1840, this commit slims `std.windows.kernel32` to only
have the functions needed by the standard library. Since this will break
projects that relied on these, I offer two solutions:

- Make an argument as to why certain functions should be added back in.
  Note that they may just be wrappers around `ntdll` APIs, which would
  go against ziglang#1840.
  If necessary I'll add them back in *and* make wrappers in
  `std.windows` for it.
- Maintain your own list of APIs. This is the option taken by bun[1],
  where they wrap functions with tracing.
- Use `zigwin32`.

I've also added TODO comments that specify which functions can be
reimplemented using `ntdll` APIs in the future.

Other changes:
- Group functions into groups (I/O, process management etc.).
- Synchronize definitions against Microsoft documentation to use the
  proper parameter types/names.
- Break all functions with parameters over multiple lines.
@The-King-of-Toasters
Copy link
Contributor Author

@andrewrk This pr has been open for a couple months now, since then a couple Windows API changes have been made. I've kept this branch up-to-date but I'd like it to be merged before I do any more changes (specifically, adding a linkat implementation). I think it would also help future contributors who want to tackle #1840.

Do you have any objections to these changes?

@andrewrk andrewrk merged commit e4f5dad into ziglang:master Jul 18, 2024
10 checks passed
@andrewrk
Copy link
Member

Sorry for the delay, I hadn't seen it until now. Looks good! Thanks for your patience on the merge.

@The-King-of-Toasters The-King-of-Toasters deleted the windows-api-refactor branch July 20, 2024 01:32
squeek502 added a commit to squeek502/zig that referenced this pull request Jul 29, 2024
In ziglang#19641, this binding changed from `[*]u16` to `LPWSTR` which made it a sentinel-terminated pointer. This introduced a compiler error in the `std.os.windows.GetModuleFileNameW` wrapper since it takes a `[*]u16` pointer. This commit changes the binding back to what it was before instead of introducing a breaking change to `std.os.windows.GetModuleFileNameW`

Related: ziglang#20858
andrewrk pushed a commit that referenced this pull request Jul 29, 2024
In #19641, this binding changed from `[*]u16` to `LPWSTR` which made it a sentinel-terminated pointer. This introduced a compiler error in the `std.os.windows.GetModuleFileNameW` wrapper since it takes a `[*]u16` pointer. This commit changes the binding back to what it was before instead of introducing a breaking change to `std.os.windows.GetModuleFileNameW`

Related: #20858
SammyJames pushed a commit to SammyJames/zig that referenced this pull request Aug 7, 2024
In ziglang#19641, this binding changed from `[*]u16` to `LPWSTR` which made it a sentinel-terminated pointer. This introduced a compiler error in the `std.os.windows.GetModuleFileNameW` wrapper since it takes a `[*]u16` pointer. This commit changes the binding back to what it was before instead of introducing a breaking change to `std.os.windows.GetModuleFileNameW`

Related: ziglang#20858
igor84 pushed a commit to igor84/zig that referenced this pull request Aug 11, 2024
In ziglang#19641, this binding changed from `[*]u16` to `LPWSTR` which made it a sentinel-terminated pointer. This introduced a compiler error in the `std.os.windows.GetModuleFileNameW` wrapper since it takes a `[*]u16` pointer. This commit changes the binding back to what it was before instead of introducing a breaking change to `std.os.windows.GetModuleFileNameW`

Related: ziglang#20858
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.

3 participants