This program is in GUI subsystem, but expects I/O with Unix console. This is a Wine specific behavior. So explicitely use unix fd instead of the inherited standard handles.
Wine-Bugs: https://bugs.winehq.org/show_bug.cgi?id=55723
Signed-off-by: Eric Pouech epouech@codeweavers.com
From: Eric Pouech epouech@codeweavers.com
This program is in GUI subsystem, but expects I/O with Unix console. This is a Wine specific behavior. So explicitely use unix fd instead of the inherited standard handles.
Wine-Bugs: https://bugs.winehq.org/show_bug.cgi?id=55723
Signed-off-by: Eric Pouech epouech@codeweavers.com --- programs/regsvr32/regsvr32.c | 47 ++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 18 deletions(-)
diff --git a/programs/regsvr32/regsvr32.c b/programs/regsvr32/regsvr32.c index 81caeb2c820..a432fa12551 100644 --- a/programs/regsvr32/regsvr32.c +++ b/programs/regsvr32/regsvr32.c @@ -27,6 +27,7 @@ #include <ole2.h> #include "regsvr32.h" #include "wine/debug.h" +#include "wine/server.h"
WINE_DEFAULT_DEBUG_CHANNEL(regsvr32);
@@ -36,12 +37,38 @@ typedef HRESULT (WINAPI *DLLINSTALL) (BOOL,LPCWSTR);
static BOOL Silent = FALSE;
+/* This program is actually defined in GUI subsystem, but still wants + * to print to Unix console if available. + */ +static void wine_console_write(const WCHAR* str) +{ + static HANDLE output_handle; + static UINT output_codepage = (UINT)-1; + + if (output_codepage == (UINT)-1) + { + wine_server_fd_to_handle(1, GENERIC_WRITE | SYNCHRONIZE, OBJ_INHERIT, &output_handle); + output_codepage = (GetFileType(output_handle) == FILE_TYPE_CHAR) ? CP_UNIXCP : CP_OEMCP; + } + if (output_handle) + { + DWORD lenA = WideCharToMultiByte(output_codepage, 0, str, -1, NULL, 0, NULL, NULL); + char *strA = malloc(lenA); + if (strA) + { + WideCharToMultiByte(output_codepage, 0, str, -1, strA, lenA, NULL, NULL); + WriteFile(output_handle, strA, lenA, NULL, FALSE); + free(strA); + } + } +} + static void WINAPIV output_write(UINT id, ...) { WCHAR fmt[1024]; va_list va_args; WCHAR *str; - DWORD len, nOut; + DWORD len;
if (Silent) return;
@@ -61,23 +88,7 @@ static void WINAPIV output_write(UINT id, ...) return; }
- /* WriteConsole fails if its output is redirected to a file. - * If this occurs, we should use an OEM codepage and call WriteFile. - */ - if (!WriteConsoleW(GetStdHandle(STD_OUTPUT_HANDLE), str, len, &nOut, NULL)) - { - DWORD lenA; - char *strA; - - lenA = WideCharToMultiByte(GetOEMCP(), 0, str, len, NULL, 0, NULL, NULL); - strA = HeapAlloc(GetProcessHeap(), 0, lenA); - if (strA) - { - WideCharToMultiByte(GetOEMCP(), 0, str, len, strA, lenA, NULL, NULL); - WriteFile(GetStdHandle(STD_OUTPUT_HANDLE), strA, lenA, &nOut, FALSE); - HeapFree(GetProcessHeap(), 0, strA); - } - } + wine_console_write(str); LocalFree(str); }
This is part of a larger change to behave as native regarding inheriting (or not) the std handles in CreateProcess [1].
We have a couple of Wine programs (regsvr32, uninstaller, winecfg, wscript...) which are compiled in the GUI subsystem (as their native counterparts are), but still requires output in unix console when run from it.
Since we no longer inherit console handle for GUIs, this output of unix console is broken (when run from command line using start.exe) (see https://bugs.winehq.org/show_bug.cgi?id=55723).
The goal of [1] is to zero the inherited std handles in CreateProcess for a child in GUI subsystem as native does. Not doing it create other problems (see https://bugs.winehq.org/show_bug.cgi?id=55439 for example).
I tried several options to handle this (also considering something that would be consistent will all the changes in [1]).
I considered several options:
* if initial process is in GUI subsystem, create a handle around fd 1 and don't create a unix console. That would work when not using start.exe. But if child has to be run with start.exe, the standard inheritance of std handles bound to unix console won't work (std handles to console are zero:ed out for GUI, see tests in [1]), so the solution would be to, in start.exe: bind unix fd 0,1,2 to handles and pass them in start.exe's CreateProcess; but only if child is GUI (if CUI we still want to keep unix console inheritance). Doable, but potentially ugly. * use AttachConsole(ATTACH_PARENT_PROCESS) in regsvr32. This works well when run with start.exe, but fails when run without (as process has no parent). This would require detecting this case of failure in AttachConsole() and fall back to creating a new unix console. An alternative would be to launch every program in CUI subsystem with start.exe. But I'm not sure we want this. * so I ended up simply reopening fd 1 (and others if needed) in regsvr32. This has the advantage of keeping ntdll and start.exe behavior as close as possible at what native does, and put the burden on each program.
Comments welcomed.
That fix shall be applied on all the Wine programs in GUI subsystems requesting I/O with unix console.
[1] https://gitlab.winehq.org/epo/wine/-/tree/mr-console?ref_type=heads
Is the lack of output actually a bug? How do you get it on Windows? I tried a few ways to run regsvr32 and /s does suppress message boxes, but does not produce any output to cmd. Error to load dlls, or failed function calls all are reported via message boxes. Same with wscript. However cscript does print to the cmd, as it should.
On Mon Nov 20 13:36:27 2023 +0000, Nikolay Sivov wrote:
Is the lack of output actually a bug? How do you get it on Windows? I tried a few ways to run regsvr32 and /s does suppress message boxes, but does not produce any output to cmd. Error to load dlls, or failed function calls all are reported via message boxes. Same with wscript. However cscript does print to the cmd, as it should.
agreed, this is not Windows behavior for regsvr32, and the target is that all output should be migrated to user32 dialogs for regsvr32.
But 1) there's seem to be some demand for not breaking the existing behavior (and ease scripting integration in Unix world), 2) I don't want to put on the critical path the changes for the migration to GUI interface.
note still that there are some others native GUI which output to std (msiexec IIRC)
Jinoh Kang (@iamahuman) commented about programs/regsvr32/regsvr32.c:
static BOOL Silent = FALSE;
+/* This program is actually defined in GUI subsystem, but still wants
- to print to Unix console if available.
- */
+static void wine_console_write(const WCHAR* str) +{
- static HANDLE output_handle;
- static UINT output_codepage = (UINT)-1;
- if (output_codepage == (UINT)-1)
- {
wine_server_fd_to_handle(1, GENERIC_WRITE | SYNCHRONIZE, OBJ_INHERIT, &output_handle);
The output handle is marked inheritable, but child processes won't know about the handle. Making it inheritable will thus lead to "handle leak."
```suggestion:-0+0 wine_server_fd_to_handle(1, GENERIC_WRITE | SYNCHRONIZE, 0, &output_handle); ```
Contrast this with other uses of `wine_server_fd_to_handle( {0,1,2}, ..., OBJ_INHERIT, &... )`[^1][^2], which explicitly uses the allocated handle for std handles in process parameters. Note that wineserver assumes that std handles are inheritable[^3].
[^1]: https://gitlab.winehq.org/wine/wine/-/blob/7c45c7c5ebb59237d568a4e5b38626422e670b63/dlls/ntdll/unix/env.c#L1382-1384 [^2]: https://gitlab.winehq.org/wine/wine/-/blob/7c45c7c5ebb59237d568a4e5b38626422e670b63/dlls/ntdll/unix/env.c#L1619 [^3]: https://gitlab.winehq.org/wine/wine/-/blob/7c45c7c5ebb59237d568a4e5b38626422e670b63/server/process.c#L1336-1347
On Mon Nov 20 14:13:21 2023 +0000, Jinoh Kang wrote:
The output handle is marked inheritable, but child processes won't know about the handle. Making it inheritable will thus lead to "handle leak."
wine_server_fd_to_handle(1, GENERIC_WRITE | SYNCHRONIZE, 0, &output_handle);Contrast this with other uses of `wine_server_fd_to_handle( {0,1,2}, ..., OBJ_INHERIT, &... )`[^1][^2], which explicitly uses the allocated handle for std handles in process parameters. Note that wineserver assumes that std handles are inheritable[^3]. [^1]: https://gitlab.winehq.org/wine/wine/-/blob/7c45c7c5ebb59237d568a4e5b38626422e670b63/dlls/ntdll/unix/env.c#L1382-1384 [^2]: https://gitlab.winehq.org/wine/wine/-/blob/7c45c7c5ebb59237d568a4e5b38626422e670b63/dlls/ntdll/unix/env.c#L1619 [^3]: https://gitlab.winehq.org/wine/wine/-/blob/7c45c7c5ebb59237d568a4e5b38626422e670b63/server/process.c#L1336-1347
The inheritance was intentional. Nevermind, sorry.
On Mon Nov 20 13:36:27 2023 +0000, eric pouech wrote:
agreed, this is not Windows behavior for regsvr32, and the target is that all output should be migrated to user32 dialogs for regsvr32. But 1) there's seem to be some demand for not breaking the existing behavior (and ease scripting integration in Unix world), 2) I don't want to put on the critical path the changes for the migration to GUI interface. note still that there are some others native GUI which output to std (msiexec IIRC)
Could we use something like `MESSAGE` (or `ERR`) macro in addition to implementing Windows behavior? That should address 1).
On Mon Nov 20 16:13:05 2023 +0000, Jacek Caban wrote:
Could we use something like `MESSAGE` (or `ERR`) macro in addition to implementing Windows behavior? That should address 1).
sure, but how do you handle the conversion to multibyte?
On Mon Nov 20 16:31:25 2023 +0000, Jinoh Kang wrote:
The inheritance was intentional. Nevermind, sorry.
no you're right, OBJ_INHERIT is of no use here.
On Mon Nov 20 16:30:33 2023 +0000, eric pouech wrote:
sure, but how do you handle the conversion to multibyte?
I was thinking about treating it other debug logs and keep it in English. As long as GUI messages are translated, it doesn't seem too bad to me.
On Mon Nov 20 21:49:02 2023 +0000, Jacek Caban wrote:
I was thinking about treating it other debug logs and keep it in English. As long as GUI messages are translated, it doesn't seem too bad to me.
so you mean:
a) move all output to graphical first
b) to keep scripting, we need a way to disable graphical output (either new command line option, or force use of null_drv) ; and output messages forced in English locale to stdout
On Tue Nov 21 09:47:59 2023 +0000, eric pouech wrote:
so you mean: a) move all output to graphical first b) to keep scripting, we need a way to disable graphical output (either new command line option, or force use of null_drv) ; and output messages forced in English locale to stdout
As Nikolay mentioned, regsvr32 already has /s which may be used to disable graphical output. For console version of script host cscript may be used instead of wscript.
If other tools don't have similar switches, I guess we could try to detect nulldrv and skip popups when it's used or something along those lines, I'm not sure.
On Tue Nov 21 10:16:07 2023 +0000, Jacek Caban wrote:
As Nikolay mentioned, regsvr32 already has /s which may be used to disable graphical output. For console version of script host cscript may be used instead of wscript. If other tools don't have similar switches, I guess we could try to detect nulldrv and skip popups when it's used or something along those lines, I'm not sure.
Another aspect is that it might be interesting in general to test our utilities on Windows, and having customizations like that would prevent it. Return code still reflects failure/success, so for scripting that might be enough?
On Tue Nov 21 16:30:14 2023 +0000, Nikolay Sivov wrote:
Another aspect is that it might be interesting in general to test our utilities on Windows, and having customizations like that would prevent it. Return code still reflects failure/success, so for scripting that might be enough?
the usual way would be to #ifndef the corresponding code ; note that MESSAGE cannot be preprocessed to nothing (not that hard to add though)
for the error code, didn't look at all of them but from Wine's regedit's exit_errorcode() function
```plaintext programs/regedit/regedit.c: exit(0); /* regedit.exe always terminates with error code zero */ ```
-
Eric Pouech -
eric pouech (＠epo)
-
Jacek Caban (＠jacek)
-
Jinoh Kang (＠iamahuman)
-
Nikolay Sivov (＠nsivov)