[PATCH v3 0/2] MR1492: Load and save application-specific console settings from the registry
On Windows, application-specific settings are stored in the registry under HKCU\Console\<path_to_app>. Our implementation of conhost.exe does not know which process is connected to it, so we need to get the full process image name before loading any application-specific console settings. To do this, we extend the server protocol to pass the process Id of the connected console process to conhost.exe. Please run tools/make_requests before merging. -- v3: conhost: Load application-specific settings using the full process image name server: Send console process ID via get_next_console_request() https://gitlab.winehq.org/wine/wine/-/merge_requests/1492
From: Hugh McMaster <hugh.mcmaster(a)outlook.com> conhost.exe needs to know the ID of the connected console process to be able to load and save application-specific console settings to and from the registry. --- server/console.c | 12 ++++++++++++ server/protocol.def | 1 + 2 files changed, 13 insertions(+) diff --git a/server/console.c b/server/console.c index 5f3f50d006f..9320c473dab 100644 --- a/server/console.c +++ b/server/console.c @@ -57,6 +57,7 @@ struct console struct thread *renderer; /* console renderer thread */ struct screen_buffer *active; /* active screen buffer */ struct console_server *server; /* console server object */ + unsigned int pid; /* console process ID */ unsigned int last_id; /* id of last created console buffer */ struct fd *fd; /* for bare console, attached input fd */ struct async_queue ioctl_q; /* ioctl queue */ @@ -538,6 +539,7 @@ static struct object *create_console(void) console->active = NULL; console->server = NULL; console->fd = NULL; + console->pid = 0; console->last_id = 0; init_async_queue( &console->ioctl_q ); init_async_queue( &console->read_q ); @@ -1565,6 +1567,14 @@ DECL_HANDLER(get_next_console_request) if (status == STATUS_PENDING) status = STATUS_INVALID_PARAMETER; if (async) { + if (!server->console->pid) + { + struct process *process = async_get_thread( async )->process; + + if (process->console && process->parent_id == current->process->parent_id) + server->console->pid = process->id; + } + iosb = async_get_iosb( async ); if (iosb->status == STATUS_PENDING) { @@ -1586,6 +1596,8 @@ DECL_HANDLER(get_next_console_request) server->busy = 0; } + reply->pid = server->console->pid; + /* if we have a blocking read ioctl in queue head and previous blocking read is still waiting, * move it to read queue for execution after current read is complete. move all blocking * ioctl at the same time to preserve their order. */ diff --git a/server/protocol.def b/server/protocol.def index 8c2fbeb4afe..038c9a4d82f 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1514,6 +1514,7 @@ enum server_fd_type unsigned int status; /* status of previous ioctl */ VARARG(out_data,bytes); /* out_data of previous ioctl */ @REPLY + unsigned int pid; /* console process ID */ unsigned int code; /* ioctl code */ unsigned int output; /* output id or 0 for input */ data_size_t out_size; /* ioctl output size */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1492
From: Hugh McMaster <hugh.mcmaster(a)outlook.com> On Windows, application-specific settings are stored in the registry under HKCU\Console\<path_to_app>. Our implementation of conhost.exe does not know which process is connected to it, so we need to get the full process image name before loading any application-specific console settings. --- programs/conhost/conhost.c | 7 ++-- programs/conhost/conhost.h | 2 ++ programs/conhost/window.c | 71 ++++++++++++++++++++++---------------- 3 files changed, 49 insertions(+), 31 deletions(-) diff --git a/programs/conhost/conhost.c b/programs/conhost/conhost.c index c3a106c9321..2cdf426e633 100644 --- a/programs/conhost/conhost.c +++ b/programs/conhost/conhost.c @@ -2716,7 +2716,7 @@ static NTSTATUS console_input_ioctl( struct console *console, unsigned int code, static NTSTATUS process_console_ioctls( struct console *console ) { size_t out_size = 0, in_size; - unsigned int code; + unsigned int code, pid; int output; NTSTATUS status = STATUS_SUCCESS; @@ -2733,6 +2733,7 @@ static NTSTATUS process_console_ioctls( struct console *console ) wine_server_add_data( req, ioctl_buffer, out_size ); wine_server_set_reply( req, ioctl_buffer, ioctl_buffer_size ); status = wine_server_call( req ); + pid = reply->pid; code = reply->code; output = reply->output; out_size = reply->out_size; @@ -2753,6 +2754,9 @@ static NTSTATUS process_console_ioctls( struct console *console ) return status; } + if (!console->init && pid) + init_window_config( console, pid ); + if (code == IOCTL_CONDRV_INIT_OUTPUT) { TRACE( "initializing output %x\n", output ); @@ -2947,7 +2951,6 @@ int __cdecl wmain(int argc, WCHAR *argv[]) if (!init_window( &console )) return 1; GetStartupInfoW( &si ); set_console_title( &console, si.lpTitle, wcslen( si.lpTitle ) * sizeof(WCHAR) ); - ShowWindow( console.win, (si.dwFlags & STARTF_USESHOWWINDOW) ? si.wShowWindow : SW_SHOW ); } return main_loop( &console, signal ); diff --git a/programs/conhost/conhost.h b/programs/conhost/conhost.h index 4464f51032f..e99fafa3f8e 100644 --- a/programs/conhost/conhost.h +++ b/programs/conhost/conhost.h @@ -75,6 +75,7 @@ struct edit_line struct console { HANDLE server; /* console server handle */ + BOOL init; /* TRUE if console has initialized */ unsigned int mode; /* input mode */ struct screen_buffer *active; /* active screen buffer */ int is_unix; /* UNIX terminal mode */ @@ -145,6 +146,7 @@ NTSTATUS change_screen_buffer_size( struct screen_buffer *screen_buffer, int new void update_console_font( struct console *console, const WCHAR *face_name, size_t face_name_size, unsigned int height, unsigned int weight ); BOOL init_window( struct console *console ); +void init_window_config( struct console *console, unsigned int pid ); void init_message_window( struct console *console ); void update_window_region( struct console *console, const RECT *update ); void update_window_config( struct console *console, BOOL delay ); diff --git a/programs/conhost/window.c b/programs/conhost/window.c index 3db4b159696..ce421eace0a 100644 --- a/programs/conhost/window.c +++ b/programs/conhost/window.c @@ -2392,33 +2392,32 @@ void update_window_region( struct console *console, const RECT *update ) update_window_config( console, TRUE ); } -BOOL init_window( struct console *console ) +void init_window_config( struct console *console, unsigned int pid) { + DWORD len, i; + HANDLE h; + WCHAR buf[256]; struct console_config config; - WNDCLASSW wndclass; STARTUPINFOW si; - CHARSETINFO ci; - static struct console_window console_window; + console->init = TRUE; - console->window = &console_window; - if (!TranslateCharsetInfo( (DWORD *)(INT_PTR)GetACP(), &ci, TCI_SRCCODEPAGE )) - return FALSE; + if (!console->window || console->no_window) return; - console->window->ui_charset = ci.ciCharset; + len = ARRAY_SIZE( buf ); + h = OpenProcess( PROCESS_QUERY_LIMITED_INFORMATION, FALSE, pid ); + QueryFullProcessImageNameW( h, 0, buf, &len ); + CloseHandle(h); - GetStartupInfoW(&si); - if (si.lpTitle) - { - size_t i, title_len = wcslen( si.lpTitle ); - if (!(console->window->config_key = malloc( (title_len + 1) * sizeof(WCHAR) ))) - return FALSE; - for (i = 0; i < title_len; i++) - console->window->config_key[i] = si.lpTitle[i] == '\\' ? '_' : si.lpTitle[i]; - console->window->config_key[title_len] = 0; - } + console->window->config_key = malloc( (len + 1) * sizeof(WCHAR) ); + + for (i = 0; i < len; i++) + console->window->config_key[i] = buf[i] == '\\' ? '_' : buf[i]; + console->window->config_key[len] = 0; load_config( console->window->config_key, &config ); + + GetStartupInfoW( &si ); if (si.dwFlags & STARTF_USECOUNTCHARS) { config.sb_width = si.dwXCountChars; @@ -2427,6 +2426,27 @@ BOOL init_window( struct console *console ) if (si.dwFlags & STARTF_USEFILLATTRIBUTE) config.attr = si.dwFillAttribute; + if (!config.face_name[0]) + set_first_font( console, &config ); + + apply_config( console, &config ); + + ShowWindow( console->win, (si.dwFlags & STARTF_USESHOWWINDOW) ? si.wShowWindow : SW_SHOW ); +} + +BOOL init_window( struct console *console ) +{ + WNDCLASSW wndclass; + CHARSETINFO ci; + + static struct console_window console_window; + + console->window = &console_window; + if (!TranslateCharsetInfo( (DWORD *)(INT_PTR)GetACP(), &ci, TCI_SRCCODEPAGE )) + return FALSE; + + console->window->ui_charset = ci.ciCharset; + wndclass.style = CS_DBLCLKS; wndclass.lpfnWndProc = window_proc; wndclass.cbClsExtra = 0; @@ -2439,17 +2459,10 @@ BOOL init_window( struct console *console ) wndclass.lpszClassName = L"WineConsoleClass"; RegisterClassW(&wndclass); - if (!CreateWindowW( wndclass.lpszClassName, NULL, - WS_OVERLAPPED|WS_CAPTION|WS_SYSMENU|WS_THICKFRAME|WS_MINIMIZEBOX| - WS_MAXIMIZEBOX|WS_HSCROLL|WS_VSCROLL, CW_USEDEFAULT, CW_USEDEFAULT, - 0, 0, 0, 0, wndclass.hInstance, console )) - return FALSE; - - if (!config.face_name[0]) - set_first_font( console, &config ); - - apply_config( console, &config ); - return TRUE; + return !!CreateWindowW( wndclass.lpszClassName, NULL, + WS_OVERLAPPED|WS_CAPTION|WS_SYSMENU|WS_THICKFRAME|WS_MINIMIZEBOX| + WS_MAXIMIZEBOX|WS_HSCROLL|WS_VSCROLL, CW_USEDEFAULT, CW_USEDEFAULT, + 0, 0, 0, 0, wndclass.hInstance, console ); } void init_message_window( struct console *console ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1492
Hi Hugh, didn't look at all the details, but several things are not clear to me: - what is the application we need refer to? it's not 100% clear its the first application attached to console, as this is not only used at conhost startup, but during all lifetime (for also saving the properties); add to make things simple, the app could have terminated, or some other could have been created - attached to the same console - according to https://devblogs.microsoft.com/commandline/understanding-windows-console-hos..., both title and process' path could be used; it's not clear why one of the two is used (load & store) (or said in another words, why the configuration based on console's title isn't sufficient?) - another question: do we need to modify the server protocol here? assuming conhost parent's process is the one of interest, NtQueryInformationProcess + ProcessBasicInformation would get you the parent pid -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1492#note_16985
Hi Eric, These are good questions.
what is the application we need refer to? it's not 100% clear its the first application attached to console, as this is not only used at conhost startup, but during all lifetime (for also saving the properties); add to make things simple, the app could have terminated, or some other could have been created - attached to the same console
As far as I can tell, Windows always loads console settings for the first process attached to conhost.exe. - Creating a new console process (CREATE_NEW_PROCESS flag) creates a new instance of conhost.exe, so registry settings are loaded as normal. - Creating a new inherited console (no process creation flags) uses the existing parent console and settings.
according to https://devblogs.microsoft.com/commandline/understanding-windows-console-hos..., both title and process' path could be used; it's not clear why one of the two is used (load & store) (or said in another words, why the configuration based on console's title isn't sufficient?)
It is true that both process path and title can be used to load and store console settings in the registry, although I've never seen any application use the title method. We can support both methods, although I think we should focus on the process path method now and add the lpTitle method later.
do we need to modify the server protocol here? assuming conhost parent's process is the one of interest, NtQueryInformationProcess + ProcessBasicInformation would get you the parent pid
`NtQueryInformationProcess` takes a HANDLE to a process as its first argument. conhost.exe doesn't have the calling process' HANDLE, nor does it know which console process it is working with. The server knows the ID of the calling process, so we can pass the PID to conhost.exe. Do note that I expect there there will be other uses for this information in conhost.exe. On Wine, conhost.exe and the console process are children of wineconsole. On Windows, conhost.exe is the child of the console process, but from I understand, Windows fakes that parent-child relationship. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1492#note_17163
This is causing a division-by-zero bug in the kernel32/console tests. I’m looking into it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1492#note_17173
On Wed Nov 23 14:38:50 2022 +0000, Hugh McMaster wrote: > Hi Eric, > These are good questions. > > what is the application we need refer to? it's not 100% clear its the > first application attached to console, as this is not only used at > conhost startup, but during all lifetime (for also saving the > properties); add to make things simple, the app could have terminated, > or some other could have been created - attached to the same console > As far as I can tell, Windows always loads console settings for the > first process attached to conhost.exe. > - Creating a new console process (CREATE_NEW_PROCESS flag) creates a new > instance of conhost.exe, so registry settings are loaded as normal. > - Creating a new inherited console (no process creation flags) uses the > existing parent console and settings. > > according to > https://devblogs.microsoft.com/commandline/understanding-windows-console-host-settings/, > both title and process' path could be used; it's not clear why one of > the two is used (load & store) (or said in another words, why the > configuration based on console's title isn't sufficient?) > It is true that both process path and title can be used to load and > store console settings in the registry, although I've never seen any > application use the title method. We can support both methods, although > I think we should focus on the process path method now and add the > lpTitle method later. > > do we need to modify the server protocol here? assuming conhost > parent's process is the one of interest, NtQueryInformationProcess + > ProcessBasicInformation would get you the parent pid > `NtQueryInformationProcess` takes a HANDLE to a process as its first > argument. conhost.exe doesn't have the calling process' HANDLE, nor does > it know which console process it is working with. The server knows the > ID of the calling process, so we can pass the PID to conhost.exe. Do > note that I expect there there will be other uses for this information > in conhost.exe. > On Wine, conhost.exe and the console process are children of > wineconsole. On Windows, conhost.exe is the child of the console > process, but from I understand, Windows fakes that parent-child relationship. for the title bits, just a wild guess: - if the console title is set via CreateProcess STARTUPINFO.lpTitle then the configuration is bound to the title - otherwise the configuration is bound to the first process path (AFAICS loading/storing configuration based on title is already present in window.c) you're right in Wine conhost / created process relationship can be tricky (to say the least): ``` $ wine cmd ``` both cmd.exe and conhost.exe are children of start.exe (and we'd expect to use cmd.exe and not start.exe for the configuration path, as they should be both attached to console) ``` $ wine programs/cmd/cmd.exe ``` conhost.exe is a child of cmd.exe (things are slightly different when allocating console at first with wineconsole) so picking the 'right' program may not be an easy task (note GetConsoleProcessList may help) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1492#note_17178
participants (3)
-
eric pouech (＠epo) -
Hugh McMaster
-
Hugh McMaster (＠hmc)