On Fri Nov 22 17:11:47 2024 +0000, eric pouech wrote:
a couple of more comments: * the /T on command line is optional (added test for that, see below) * the test that you commented out, is a left over from earlier version of the patch where the input handle was a pipe. it does work properly now that you pass console handle as input handle * so you should test after validating the command line arguments that the input handle is really a console handle (VerifyConsoleIoHandle(GetStdHandle(STD_INPUT_HANDLE)) (as windows does.... "timeout \< nul" in cmd prompt shows the relevant error) * I believe it's better in run_timeout_stdin to redirect output/error handle to NULL as we don't care for the actual output (otherwise it cripples the output) * I toyed a bit the MR and added tests for ctrl-c handling (see attached file, also includes improvements for above items). Feel free to include into your MR [patch](/uploads/fe2608693e811eeec8e2f42c2cc9ce87/patch) * Actually, the STATUS_CONTROL_C_EXIT exit code is not generated by timeout.exe but by the default handlers in kernelbase. Wine doesn't do this properly yet (I'll send another MR to fix that) * So I recommend only setting the ctrl_c hander when the /NOBREAK is present thank's for the patch. I apply that but unfortunatly I not add any console verification with
if (VerifyConsoleIoHandle(GetStdHandle(STD_INPUT_HANDLE))
...
because it work under wine but not under windows. And If I do ``` if (!_isatty(_fileno(stdin))) ``` it works under windows but not under wine -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6869#note_88553