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