[PATCH v2 0/3] MR5462: Ucrtbase's stderr is now always unbuffered.
Starting with late Windows 10 version, in ucrtbase, stderr is now unbuffered whatever the type of the underlying fd (previously, native only set it to unbuffered when attached to character fd (console, NUL...)). This serie adds also tests for msvcrt & ucrtbase to show the discrepancies. Note: ucrtbase's tests also include a reversed engineered structure layout for FILE. _get_stream_buffer_pointers already gives base, ptr and cnt, and toying with setvbuf gave easily the rest of the fields. Didn't spend time in guessing the flags meaning: which look different from msvcrt's. Related to https://bugs.winehq.org/show_bug.cgi?id=56459 @piotr: I can share the details (the access to the test result in the bug report is limited). -- v2: msvcrt: Let stderr be always be unbuffered. ucrtbase/tests: Add tests for checking buffering state of standard streams. msvcrt/tests: Add tests for check buffering state of standard streams. https://gitlab.winehq.org/wine/wine/-/merge_requests/5462
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/msvcrt/tests/file.c | 93 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/dlls/msvcrt/tests/file.c b/dlls/msvcrt/tests/file.c index 1ac6cfe3182..e680f9ce2b8 100644 --- a/dlls/msvcrt/tests/file.c +++ b/dlls/msvcrt/tests/file.c @@ -3059,6 +3059,96 @@ static void test_ioinfo_flags(void) free(tempf); } +static void test_std_buffer_state( const char *selfname ) +{ + char cmdline[MAX_PATH]; + STARTUPINFOA startup = {.cb = sizeof(startup), .dwFlags = 0}; + PROCESS_INFORMATION proc; + DWORD exit_code; + BOOL ret; + + /* use child process to temper with std streams and only use ok() in parent process */ + sprintf( cmdline, "%s file stdbuf", selfname); + ret = CreateProcessA( NULL, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &proc ); + ok(ret, "Couldn't start process '%s'\n", cmdline); + ret = WaitForSingleObject( proc.hProcess, 30000 ); + ok(ret == WAIT_OBJECT_0, "Unexpected process termination\n"); + ret = GetExitCodeProcess( proc.hProcess, &exit_code ); + ok( ret, "Unexpected result\n" ); + ok( exit_code == 7, "Unexpected exit code %lx\n", exit_code); + CloseHandle( proc.hProcess ); + CloseHandle( proc.hThread ); +} + +static BOOL create_file( const char *filename, const char *content ) +{ + FILE *file; + int len; + + if (!(file = fopen( filename, "wb" ))) return FALSE; + len = fputs( content, file ); + fclose( file ); + return len >= 0; +} + +static unsigned gather_buffer_state( FILE *f ) +{ + unsigned ret = 0; + int fd = _fileno( f ); + __int64 pos; + char ch; + + /* these operations shall force buffer creation (if required) */ + switch (fd) + { + case 0: + if (fscanf( f, "%c", &ch ) != 1 || (pos = _telli64( fd )) < 0) + ret |= 1u << 7; + else if (pos > 2) + ret |= 1u << fd; + break; + case 1: + case 2: + if (fprintf( f, "a" ) != 1 || (pos = _telli64( fd )) < 0) + ret |= 1u << 7; + else if (pos == 0) + ret |= 1u << fd; + break; + default: + ret |= 1u << 7; + break; + } + return ret; +} + +static void test_std_buffer_state_child( int argc, char **argv ) +{ + unsigned exit_code = 0; + + /* return: + * - bit7 set in case of error; + * - bit0,1,2 set if fd 0,1,2 is buffered + */ + if (!create_file( "file0.tmp", "content\n" ) || + !freopen( "file0.tmp", "r", stdin ) || + !freopen( "file1.tmp", "w", stdout ) || + !freopen( "file2.tmp", "w", stderr )) + exit_code |= 1u << 7; + + exit_code |= gather_buffer_state( stdin ); + exit_code |= gather_buffer_state( stdout ); + exit_code |= gather_buffer_state( stderr ); + + fclose( stdin ); + fclose( stdout ); + fclose( stderr ); + + if (!DeleteFileA( "file0.tmp" ) || !DeleteFileA( "file1.tmp" ) || !DeleteFileA( "file2.tmp" )) + exit_code |= 1u << 7; + + ExitProcess( exit_code ); +} + START_TEST(file) { int arg_c; @@ -3079,6 +3169,8 @@ START_TEST(file) test_pipes_child(arg_c, arg_v); else if (strcmp(arg_v[2], "stdin") == 0) test_invalid_stdin_child(); + else if (strcmp(arg_v[2], "stdbuf") == 0) + test_std_buffer_state_child(arg_c, arg_v); else ok(0, "invalid argument '%s'\n", arg_v[2]); return; @@ -3135,6 +3227,7 @@ START_TEST(file) test_fopen_hints(); test_open_hints(); test_ioinfo_flags(); + test_std_buffer_state(arg_v[0]); /* Wait for the (_P_NOWAIT) spawned processes to finish to make sure the report * file contains lines in the correct order -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5462
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/ucrtbase/tests/Makefile.in | 3 +- dlls/ucrtbase/tests/file.c | 160 ++++++++++++++++++++++++++++++++ 2 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 dlls/ucrtbase/tests/file.c diff --git a/dlls/ucrtbase/tests/Makefile.in b/dlls/ucrtbase/tests/Makefile.in index cd01a3aacb9..d6ea94bc70f 100644 --- a/dlls/ucrtbase/tests/Makefile.in +++ b/dlls/ucrtbase/tests/Makefile.in @@ -1,10 +1,11 @@ TESTDLL = ucrtbase.dll -IMPORTS = ucrtbase +IMPORTS = ucrtbase version EXTRADEFS = -fno-builtin SOURCES = \ cpp.c \ environ.c \ + file.c \ misc.c \ printf.c \ scanf.c \ diff --git a/dlls/ucrtbase/tests/file.c b/dlls/ucrtbase/tests/file.c new file mode 100644 index 00000000000..69e51133528 --- /dev/null +++ b/dlls/ucrtbase/tests/file.c @@ -0,0 +1,160 @@ +/* + * Unit test suite for file functions + * + * Copyright 2024 Eric Pouech for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include "wine/test.h" +#include <winver.h> + +static BOOL detect_too_old_ucrtbase( void ) +{ + DWORD size; + BYTE *buffer; + VS_FIXEDFILEINFO *info; + UINT size_info; + BOOL ret = FALSE; + + if ((size = GetFileVersionInfoSizeW( L"ucrtbase.dll", NULL )) && (buffer = malloc( size ))) + { + if (GetFileVersionInfoW( L"ucrtbase.dll", 0, size, buffer ) && + VerQueryValueW( buffer, L"\\", (LPVOID*)&info, &size_info ) ) + { + /* last known not supporting version is 10.0.17763.719 */ + /* first known supporting version is 10.0.19041.789 */ + ret = (info->dwProductVersionMS == (10u << 16)) && info->dwProductVersionLS < ((19041 << 16) | 789); + } + free( buffer ); + } + return ret; +} + +static void test_std_buffer_state( const char *selfname ) +{ + char cmdline[MAX_PATH]; + STARTUPINFOA startup = {.cb = sizeof(startup), .dwFlags = 0}; + PROCESS_INFORMATION proc; + DWORD exit_code; + BOOL ret; + + /* Win7, 8 and early Win10 keep stderr bufferred on files */ + if (!winetest_platform_is_wine && detect_too_old_ucrtbase()) + { + skip("Too old ucrtbase version\n"); + return; + } + + /* use child process to temper with std streams and only use ok() in parent process */ + sprintf( cmdline, "%s file stdbuf", selfname); + ret = CreateProcessA( NULL, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &proc ); + ok(ret, "Couldn't start process '%s'\n", cmdline); + ret = WaitForSingleObject( proc.hProcess, 30000 ); + ok(ret == WAIT_OBJECT_0, "Unexpected process termination\n"); + ret = GetExitCodeProcess( proc.hProcess, &exit_code ); + ok( ret, "Unexpected result\n" ); + todo_wine + ok( exit_code == 3, "Unexpected exit code %lx\n", exit_code); + CloseHandle( proc.hProcess ); + CloseHandle( proc.hThread ); +} + +static BOOL create_file( const char *filename, const char *content ) +{ + FILE *file; + int len; + + if (!(file = fopen( filename, "wb" ))) return FALSE; + len = fputs( content, file ); + fclose( file ); + return len >= 0; +} + +static unsigned gather_buffer_state( FILE *f ) +{ + unsigned ret = 0; + int fd = _fileno( f ); + __int64 pos; + char ch; + + /* these operations shall force buffer creation (if required) */ + switch (fd) + { + case 0: + if (fscanf( f, "%c", &ch ) != 1 || (pos = _telli64( fd )) < 0) + ret |= 1u << 7; + else if (pos > 2) + ret |= 1u << fd; + break; + case 1: + case 2: + if (fprintf( f, "a" ) != 1 || (pos = _telli64( fd )) < 0) + ret |= 1u << 7; + else if (pos == 0) + ret |= 1u << fd; + break; + default: + ret |= 1u << 7; + break; + } + return ret; +} + +static void test_std_buffer_state_child( int argc, char **argv ) +{ + unsigned exit_code = 0; + + /* return: + * - bit7 set in case of error; + * - bit0,1,2 set if fd 0,1,2 is buffered + */ + if (!create_file( "file0.tmp", "content\n" ) || + !freopen( "file0.tmp", "r", stdin ) || + !freopen( "file1.tmp", "w", stdout ) || + !freopen( "file2.tmp", "w", stderr )) + exit_code |= 1u << 7; + + exit_code |= gather_buffer_state( stdin ); + exit_code |= gather_buffer_state( stdout ); + exit_code |= gather_buffer_state( stderr ); + + fclose( stdin ); + fclose( stdout ); + fclose( stderr ); + + if (!DeleteFileA( "file0.tmp" ) || !DeleteFileA( "file1.tmp" ) || !DeleteFileA( "file2.tmp" )) + exit_code |= 1u << 7; + + ExitProcess( exit_code ); +} + +START_TEST( file ) +{ + int arg_c; + char** arg_v; + + arg_c = winetest_get_mainargs( &arg_v ); + + if (arg_c >= 3) + { + if (strcmp( arg_v[2], "stdbuf" ) == 0) + test_std_buffer_state_child( arg_c, arg_v ); + else + ok( 0, "invalid argument '%s'\n", arg_v[2] ); + return; + } + test_std_buffer_state( arg_v[0] ); +} -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5462
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/msvcrt/file.c | 6 ++++++ dlls/ucrtbase/tests/file.c | 1 - 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/dlls/msvcrt/file.c b/dlls/msvcrt/file.c index a3a872d4be3..24cca6fb63e 100644 --- a/dlls/msvcrt/file.c +++ b/dlls/msvcrt/file.c @@ -830,9 +830,15 @@ int CDECL _isatty(int fd) /* INTERNAL: Allocate stdio file buffer */ static BOOL msvcrt_alloc_buffer(FILE* file) { +#if _MSVCR_VER >= 140 + if((file->_file==STDOUT_FILENO && _isatty(file->_file)) + || file->_file == STDERR_FILENO) + return FALSE; +#else if((file->_file==STDOUT_FILENO || file->_file==STDERR_FILENO) && _isatty(file->_file)) return FALSE; +#endif file->_base = calloc(1, MSVCRT_INTERNAL_BUFSIZ); if(file->_base) { diff --git a/dlls/ucrtbase/tests/file.c b/dlls/ucrtbase/tests/file.c index 69e51133528..686d327a9a8 100644 --- a/dlls/ucrtbase/tests/file.c +++ b/dlls/ucrtbase/tests/file.c @@ -66,7 +66,6 @@ static void test_std_buffer_state( const char *selfname ) ok(ret == WAIT_OBJECT_0, "Unexpected process termination\n"); ret = GetExitCodeProcess( proc.hProcess, &exit_code ); ok( ret, "Unexpected result\n" ); - todo_wine ok( exit_code == 3, "Unexpected exit code %lx\n", exit_code); CloseHandle( proc.hProcess ); CloseHandle( proc.hThread ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5462
V2: - removed code for testing various handle types (only doing file for now) and moved FILE creation to child process - using position from file descriptor to detect if buffering is enabled - didn't find any new function in UCRTBASE to distinguish for ucrtbase with buffering of stderr disabled; so ended up using ucrtbase version information (didn't find anything better, if someone has a better idea, feel free to jump in) - kept the subprocess & reopen because wine/test.h changes stdout buffer state -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5462#note_67230
participants (2)
-
Eric Pouech -
eric pouech (＠epo)