Skip to content

[windows] file_write: require a console, not just a character device - #1374

Open
dimensionscape wants to merge 1 commit into
HaxeFoundation:masterfrom
dimensionscape:fix/file-write-nul-device
Open

[windows] file_write: require a console, not just a character device#1374
dimensionscape wants to merge 1 commit into
HaxeFoundation:masterfrom
dimensionscape:fix/file-write-nul-device

Conversation

@dimensionscape

Copy link
Copy Markdown
Contributor

_isatty is true for any character device, so a write to NUL took the WriteConsoleW path. WriteConsoleW fails on a non-console handle and the file_error after it throws from inside a GC-free zone, so the process dies instead of reporting anything:

sys.io.File.write("NUL").writeString("x")
prog.exe >NUL

GetConsoleMode only succeeds on a real console, and __hxcpp_print already uses it. _isatty stays as the first check so files and pipes take the same fwrite path they did before.

Came in with 1544ff5 (#1307), which fixed utf console output. That part is unchanged here.

_isatty is true for any character device, so a write to NUL took the
WriteConsoleW path. WriteConsoleW fails on a non-console handle and the
file_error after it throws from inside a GC-free zone, so the process dies
instead of reporting anything:

    sys.io.File.write("NUL").writeString("x")
    prog.exe >NUL

GetConsoleMode only succeeds on a real console, and __hxcpp_print already uses
it. _isatty stays as the first check so files and pipes take the same fwrite
path they did before.

Came in with 1544ff5 (HaxeFoundation#1307), which fixed utf console output. That part is
unchanged here.
Comment thread src/hx/libs/std/File.cpp
// _isatty is true for ANY character device - NUL, a serial port, a printer - and none of
// those accept WriteConsoleW. Only a real console has a console mode.
DWORD console_mode;
if (_isatty(_fileno(f->io)) && GetConsoleMode((HANDLE)_get_osfhandle(_fileno(f->io)), &console_mode)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we keep the _isatty check here? can we just rely on GetConsoleMode?

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.

2 participants