Skip to content

Add SetConsoleMode to windows.kernel32 #15039

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

Closed
wants to merge 1 commit into from

Conversation

Kiyoshi364
Copy link
Contributor

Hi,

I've read that windows.kernel32's functions are "lazily added" to stdlib.

I would like to use SetConsoleMode in my project.
I have already patched stdlib in my local machine and looks like it works.

@kubkon
Copy link
Member

kubkon commented Mar 21, 2023

Hi, and thank you for your contribution! Our general direction when it comes to Windows system API is to rely mainly on direct calls into the ntdll library. As a result, we strive to reimplement as much of kernel32 functionality ourselves as possible. Would you like to try and do that for the added function in this PR if possible? If so, I can provide some links that might be of use.

@Kiyoshi364
Copy link
Contributor Author

I'd like to try. If it's easy enough, should I reimplement GetConsoleMode too?
Should I make a new pull request or keep it here?

@kubkon
Copy link
Member

kubkon commented Mar 21, 2023

I'd like to try. If it's easy enough, should I reimplement GetConsoleMode too?
Should I make a new pull request or keep it here?

My suggestion is to first focus on one, and if it goes well you are more than welcome to tackle others.

@kubkon
Copy link
Member

kubkon commented Mar 21, 2023

@squeek502
Copy link
Collaborator

squeek502 commented Mar 22, 2023

This is almost guaranteed to run into the same problems encountered in #14411 (specifically #14411 (comment) and #14411 (comment)) when it comes to a ntdll implementation. Running NtTrace on a program that calls SetConsoleMode shows that it also uses NtDeviceIoControlFile internally, meaning what it provides/expects will likely be implementation defined and Windows/React/Wine will all use different input structs for the same methods.

The good news is that if you just want to use SetConsoleMode in your program, there's no need for it to be in the standard library, you can just add the binding into your code directly and it'll work fine. For example, here's the test program I used:

const std = @import("std");

pub extern "kernel32" fn SetConsoleMode(in_hConsoleHandle: std.os.windows.HANDLE, in_dwMode: std.os.windows.DWORD) callconv(std.os.windows.WINAPI) std.os.windows.BOOL;

pub fn main() !void {
    _ = SetConsoleMode(std.io.getStdIn().handle, 0x10);
}

(and here's a larger / real-world example of using many win32 bindings not in the standard library)

@Kiyoshi364
Copy link
Contributor Author

Thanks for the links and the example showing that I don't need to patch the stdlib.
I'm new to "Windows stuff".
Also, I'm using this as a reference: https://www.geoffchappell.com/studies/windows/win32/ntdll/api/index.htm

@Kiyoshi364
Copy link
Contributor Author

This is what I've done so far. index.txt[0] (a c file) is a file with definitions copied from reactos repo. main.txt[1] (a zig file) is my attempt to implement SetConsoleMode (there is also a GetConsoleMode, because they are almost equal). Calls to fn todo(_: anytype) noreturn marks where things are missing.

What is missing:

  • ConsolepSetMode/ConsolepGetMode: an u16 that indicates a function to be called. grep couldn't find them
  • Return type of NtCurrentTeb (named TEB): different sources kinda describes in a different way. It is needed to get a ConsoleHandle
  • Implementation for BaseSetLastError: this is for the caller to get the error with GetLastError

Theoretically, I could copy the entire TEB and then translate-c it to zig, but I'm not sure if it's what it should be done and I kinda feel lazy to do that.
Other idea is to "hardcode" the offsets to deref from TEB to ConsoleHandle. It sounds awful but could work out.

If anyone has suggestions (or values/implementations), I could put more work into it.
Otherwise, I'll leave it there hoping for someone to pick up from where I left.

As an additional info:

  • I don't know where in stdlib each peace of code would go to (example: SetConsoleMode goes to windows.zig; CSR_API_MESSAGE goes to windows/ntdll.zig)
  • Because of not having a definition for TEB, I can't compile neither test the code
  • Console_Api_Message.Data's union has many more possible values, but they are not there because there are many possibilities and the file would have many "unused" definitions
  • Linked files are (.txt) because Github doesn't like .c and .zig

[0] index.txt
[1] main.txt

@squeek502
Copy link
Collaborator

First, to answer some questions:

  • The ConsoleHandle can be gotten via std.os.windows.peb().ProcessParameters.ConsoleHandle
  • Win32 definition of ConsolepSetMode can be found here, but note that it may be implementation defined (i.e. ReactOS/wine may use different values). The Win32 value in this case would be (1 << 24) + 2, while the ReactOS value seems to be 529 (see here)
  • BaseSetLastError may not be necessary, as the goal doesn't need to be a perfect 1:1 reimplementation of the kernel32 function, but instead it can be a Zig-ified function that achieves a similar result (see Implement getMaxRss for Windows #15035 (comment))
  • See Replace kernel32.SetConsoleOutputCP with ntdll-implemented version mlugg/zig#1 for a similar type of function I wrote to avoid kernel32.SetConsoleOutputCP (this is from some reverse engineering of what happens on Windows, though, so it won't work with ReactOS/Wine)

Second, I think it's worth offering some warning:

@Kiyoshi364
Copy link
Contributor Author

Currently, I know that I don't need to patch std lib to my program to work. I'm doing this because I was asked to and it appeared to give me some knowledge of how "windows stuff" works.

I'm not sure how useful "my implementation" would be (I'm guessing not that useful, basing on what you said).
If it comes out to be inaccurate, I think it won't be that hard to continue from here.

I'm planing on closing this, thanks again for all links and suggestions.

(I'm not sure if closing disallows anything important, like any closing thoughts.
If someone has "the power" to close this, I think it is ok to do so.
If it is ok for me to close this, I can close with "Close with comment".)

@kubkon
Copy link
Member

kubkon commented Mar 28, 2023

Until such time we gather compelling evidence we need such a wrapper, I am closing this PR for now. Feel free to re-open or submit a new one if you disagree or find a compelling use case!

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