[PATCH 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). -- 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 | 271 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 271 insertions(+) diff --git a/dlls/msvcrt/tests/file.c b/dlls/msvcrt/tests/file.c index 1ac6cfe3182..81d0c34a397 100644 --- a/dlls/msvcrt/tests/file.c +++ b/dlls/msvcrt/tests/file.c @@ -3059,6 +3059,274 @@ static void test_ioinfo_flags(void) free(tempf); } +static unsigned __cdecl winetest_log(const char *format, ...) +{ + const char *logname = "file.log"; + va_list args; + FILE *log; + unsigned ret = -1; + + if (!strcmp( format, "delete" )) + return DeleteFileA(logname); + if (!strcmp( format, "trace" )) + { + if ((log = fopen( logname, "r" ))) + { + char tmp[256]; + + while (fgets( tmp, ARRAY_SIZE(tmp), log )) + trace( "%s", tmp ); + fclose( log ); + return 1; + } + return 0; + } + va_start( args, format ); + if ((log = fopen( logname, "a" ))) + { + ret = vfprintf( log, format, args ); + fclose( log ); + } + va_end( args ); + return ret; +} + +static void create_file(const char *filename, const char *content) +{ + DWORD ret; + HANDLE file = CreateFileA( filename, GENERIC_WRITE, + FILE_SHARE_WRITE, NULL, CREATE_ALWAYS, 0, NULL ); + ok( file != INVALID_HANDLE_VALUE, "Couldn't create file\n" ); + if (content) + { + DWORD nw; + ret = WriteFile( file, content, strlen( content ), &nw, NULL ); + ok( ret && nw == strlen( content ), "Couldn't write file\n" ); + } + CloseHandle( file ); +} + +enum buffer_state +{ + BS_NO_BUFFER, + BS_CHAR_BUFFER, + BS_FULL_BUFFER, + BS_ERROR, +}; + +static enum buffer_state get_buffer_state(FILE *f) +{ + unsigned bufsize = 0xffffffff; + char *base = NULL; + char *charbuf = (char*)(f - 1); /* so that it's not inside f structure */ + + bufsize = f->_bufsiz; + base = f->_base; + charbuf = (char*)&f->_charbuf; + + /* we could expect bufsize to be 0, but it's sometimes 2 */ + if (base == NULL && bufsize != 4096) return BS_NO_BUFFER; + if (base == charbuf && bufsize == 2) return BS_CHAR_BUFFER; + if (base != NULL && bufsize == 4096) return BS_FULL_BUFFER; + winetest_log( "Can't guess buffer state: bufsize=%04u base=%p\n", bufsize, base ); + + return BS_ERROR; +} + +static BOOL check_buffer_state_discovery(void) +{ + static char file_test_buffer[4096]; + unsigned cond = 0; + FILE *file_test = fopen( "foo.tmp", "w" ); + errno_t err; + + if (!file_test) return FALSE; + + winetest_log("delete"); + + if (get_buffer_state( file_test ) == BS_NO_BUFFER) cond++; + err = setvbuf( file_test, NULL, _IONBF, 0 ); + ok( err == 0, "setvbuf failed\n" ); + if (get_buffer_state( file_test ) == BS_CHAR_BUFFER) cond++; + err = setvbuf( file_test, file_test_buffer, _IOFBF, sizeof(file_test_buffer) ); + ok( err == 0, "setvbuf failed\n" ); + if (get_buffer_state( file_test ) == BS_FULL_BUFFER) cond++; + err = setvbuf( file_test, file_test_buffer, _IOLBF, sizeof(file_test_buffer) ); + ok( err == 0, "setvbuf failed\n" ); + if (get_buffer_state( file_test ) == BS_FULL_BUFFER) cond++; + fclose( file_test ); + + if (cond != 4) + { + win_skip("Incompatible Windows platform, skipping test\n"); + trace("DLL2 ucrt=%u msvcrt=%u wine=%u\n", + GetModuleHandleA("ucrtbase.dll") != NULL, + GetModuleHandleA("msvcrt.dll") != NULL, + winetest_platform_is_wine); + winetest_log("trace"); + return FALSE; + } + return TRUE; +} + +/* We could test more types: + * - pipes: + * - console: we assume that NUL and console behave similary (they are of FILE_TYPE_CHAR) + */ +enum handle_creation {H_FILE, H_NUL, H_REOPEN = 0x10, H_KIND_MASK = 0x0F}; + +static void test_std_buffer_state(const char *selfname) +{ + static struct + { + /* input */ + unsigned type; + /* output */ + enum buffer_state std_buffer_state[3]; + BOOL with_todo; + } + test_file_type[] = + { + /* inheriting from parent */ + /* Note: wine/test.h calls set_vbuf(_IONBF) (even in child) + * hence the result BS_CHAR_BUFFER for stdout without reopen + */ + {H_FILE, {BS_FULL_BUFFER, BS_CHAR_BUFFER, BS_FULL_BUFFER}}, + {H_NUL, {BS_FULL_BUFFER, BS_CHAR_BUFFER, BS_NO_BUFFER}}, + /* reopen in child */ + {H_FILE | H_REOPEN, {BS_FULL_BUFFER, BS_FULL_BUFFER, BS_FULL_BUFFER}}, + {H_NUL | H_REOPEN, {BS_FULL_BUFFER, BS_NO_BUFFER, BS_NO_BUFFER}}, + }; + char cmdline[MAX_PATH]; + SECURITY_ATTRIBUTES sa = {.nLength = sizeof(sa), .lpSecurityDescriptor = NULL, .bInheritHandle = TRUE}; + STARTUPINFOA startup; + PROCESS_INFORMATION proc; + DWORD exit_code, expected_exit_code; + unsigned i; + BOOL ret; + + /* check that get_buffer_state is coherent */ + if (!check_buffer_state_discovery()) return; + + for (i = 0; i < ARRAY_SIZE(test_file_type); i++) + { + winetest_push_context( "stdbuf[%d]", i ); + memset( &startup, 0, sizeof(startup) ); + startup.cb = sizeof(startup); + + winetest_log("delete"); + + switch ((int)test_file_type[i].type) + { + case H_FILE: + /* so that we can read from stdin */ + create_file( "file0.tmp", "first content\r\n" ); + startup.hStdInput = CreateFileA( "file0.tmp", GENERIC_READ, + FILE_SHARE_READ, &sa, OPEN_EXISTING, 0, NULL ); + startup.hStdOutput = CreateFileA( "file1.tmp", GENERIC_WRITE, + FILE_SHARE_WRITE, &sa, CREATE_ALWAYS, 0, NULL ); + startup.hStdError = CreateFileA( "file2.tmp", GENERIC_WRITE, + FILE_SHARE_WRITE, &sa, CREATE_ALWAYS, 0, NULL ); + startup.dwFlags = STARTF_USESTDHANDLES; + break; + case H_NUL: + startup.hStdInput = CreateFileA( "NUL", GENERIC_READ, + FILE_SHARE_READ, &sa, OPEN_EXISTING, 0, NULL ); + startup.hStdOutput = CreateFileA( "NUL", GENERIC_WRITE, + FILE_SHARE_WRITE, &sa, CREATE_ALWAYS, 0, NULL ); + startup.hStdError = CreateFileA( "NUL", GENERIC_WRITE, + FILE_SHARE_WRITE, &sa, CREATE_ALWAYS, 0, NULL ); + startup.dwFlags = STARTF_USESTDHANDLES; + break; + case H_FILE | H_REOPEN: + create_file( "file0.tmp", "first content\r\n" ); + break; + case H_NUL | H_REOPEN: + break; + default: ok(0, "\n"); break; + } + + sprintf( cmdline, "%s file stdbuf %#x", selfname, test_file_type[i].type ); + ret = CreateProcessA( NULL, cmdline, NULL, NULL, (startup.dwFlags & STARTF_USESTDHANDLES) != 0, 0, NULL, NULL, &startup, &proc ); + ok(ret, "Couldn't create child process '%s' %p %p %p: %lu\n", cmdline, startup.hStdInput, startup.hStdOutput, startup.hStdError, GetLastError()); + 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" ); + expected_exit_code = test_file_type[i].std_buffer_state[0] << 0 | + test_file_type[i].std_buffer_state[1] << 2 | + test_file_type[i].std_buffer_state[2] << 4; + todo_wine_if( test_file_type[i].with_todo ) + ok( exit_code == expected_exit_code, "Unexpected buffer state %lx: stdin=%lx/%x, stdout=%lx/%x, stderr=%lx/%x\n", + exit_code >> 6, + (exit_code >> 0) & 3, test_file_type[i].std_buffer_state[0], + (exit_code >> 2) & 3, test_file_type[i].std_buffer_state[1], + (exit_code >> 4) & 3, test_file_type[i].std_buffer_state[2] ); + + CloseHandle( proc.hProcess ); + CloseHandle( proc.hThread ); + + winetest_log("trace"); + + switch ((int)test_file_type[i].type) + { + case H_FILE: + case H_NUL: + CloseHandle( startup.hStdInput ); + CloseHandle( startup.hStdOutput ); + CloseHandle( startup.hStdError ); + break; + default: + break; + } + DeleteFileA( "file0.tmp" ); + DeleteFileA( "file1.tmp" ); + DeleteFileA( "file2.tmp" ); + + winetest_pop_context(); + } +} + +static void test_std_buffer_state_child(int argc, char **argv) +{ + unsigned flags = 0; + unsigned ret = 0; + char ch; + + if (argc >= 4) flags = strtol( argv[3], NULL, 0 ); + + if (flags & H_REOPEN) + { + switch (flags & H_KIND_MASK) + { + case H_FILE: + if (!freopen( "file0.tmp", "r", stdin )) ret |= 1 << 6; + if (!freopen( "file1.tmp", "w", stdout )) ret |= 1 << 6; + if (!freopen( "file2.tmp", "w", stderr )) ret |= 1 << 6; + break; + case H_NUL: + if (!freopen( "NUL", "r", stdin )) ret |= 1 << 6; + if (!freopen( "NUL", "w", stdout )) ret |= 1 << 6; + if (!freopen( "NUL", "w", stderr )) ret |= 1 << 6; + break; + default: + ret |= 1 << 6; + break; + } + } + + /* these operations shall force buffer creation (if required) */ + fscanf( stdin, "%c", &ch ); + fprintf( stdout, "f" ); + fprintf( stderr, "f" ); + + ret |= get_buffer_state( stdin ) << 0; + ret |= get_buffer_state( stdout ) << 2; + ret |= get_buffer_state( stderr ) << 4; + + ExitProcess( ret ); +} + START_TEST(file) { int arg_c; @@ -3079,6 +3347,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 +3405,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 | 1 + dlls/ucrtbase/tests/file.c | 352 ++++++++++++++++++++++++++++++++ 2 files changed, 353 insertions(+) create mode 100644 dlls/ucrtbase/tests/file.c diff --git a/dlls/ucrtbase/tests/Makefile.in b/dlls/ucrtbase/tests/Makefile.in index cd01a3aacb9..ea9e589113c 100644 --- a/dlls/ucrtbase/tests/Makefile.in +++ b/dlls/ucrtbase/tests/Makefile.in @@ -5,6 +5,7 @@ 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..97f2f82ff75 --- /dev/null +++ b/dlls/ucrtbase/tests/file.c @@ -0,0 +1,352 @@ +/* + * 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 <stdlib.h> +#include <stdio.h> +#include <fcntl.h> +#include <share.h> +#include <sys/stat.h> +#include <io.h> +#include <direct.h> +#include <windef.h> +#include <winbase.h> +#include <winnls.h> +#include <winreg.h> +#include <process.h> +#include <locale.h> +#include <winternl.h> + +static unsigned __cdecl winetest_log(const char *format, ...) +{ + const char *logname = "file.log"; + va_list args; + FILE *log; + unsigned ret = -1; + + if (!strcmp( format, "delete" )) + return DeleteFileA( logname ); + if (!strcmp( format, "trace" )) + { + if ((log = fopen( logname, "r" ))) + { + char tmp[256]; + + while (fgets( tmp, ARRAY_SIZE(tmp), log )) + trace( "%s", tmp ); + fclose( log ); + return 1; + } + return 0; + } + va_start( args, format ); + if ((log = fopen( logname, "a" ))) + { + ret = vfprintf( log, format, args ); + fclose( log ); + } + va_end( args ); + return ret; +} + +static void create_file(const char *filename, const char *content) +{ + DWORD ret; + HANDLE file = CreateFileA( filename, GENERIC_WRITE, + FILE_SHARE_WRITE, NULL, CREATE_ALWAYS, 0, NULL ); + ok( file != INVALID_HANDLE_VALUE, "Couldn't create file\n" ); + if (content) + { + DWORD nw; + ret = WriteFile( file, content, strlen( content ), &nw, NULL ); + ok( ret && nw == strlen( content ), "Couldn't write file\n" ); + } + CloseHandle( file ); +} + +enum buffer_state +{ + BS_NO_BUFFER, + BS_CHAR_BUFFER, + BS_FULL_BUFFER, + BS_ERROR, +}; + +struct UCRT_FILE +{ + char *_ptr; + char *_base; + int _cnt; + int _flags; + int _file; + int _charbuf; + int _bufsiz; +}; + +static enum buffer_state get_buffer_state(FILE *f) +{ + unsigned bufsize = 0xffffffff; + char *base = NULL; + char *charbuf = (char*)(f - 1); /* so that it's not inside f structure */ + + if (!f) return BS_ERROR; + + if (winetest_platform_is_wine) + { + bufsize = f->_bufsiz; + base = f->_base; + charbuf = (char*)&f->_charbuf; + } + else + { + struct UCRT_FILE *uf = (struct UCRT_FILE*)f; + + bufsize = uf->_bufsiz; + base = uf->_base; + charbuf = (char*)&uf->_charbuf; + } + /* we could expect bufsize to be 0, but it's sometimes 2 */ + if (base == NULL && bufsize != 4096) return BS_NO_BUFFER; + if (base == charbuf && bufsize == 2) return BS_CHAR_BUFFER; + if (base != NULL && bufsize == 4096) return BS_FULL_BUFFER; + winetest_log( "Can't guess buffer state: bufsize=%04u base=%p\n", bufsize, base ); + return BS_ERROR; +} + +static BOOL check_buffer_state_discovery(void) +{ + static char file_test_buffer[4096]; + unsigned cond = 0; + FILE *file_test = fopen( "foo.tmp", "w" ); + errno_t err; + + /* check that get_buffer_state is coherent */ + if (!file_test) return FALSE; + + winetest_log("delete"); + + if (get_buffer_state( file_test ) == BS_NO_BUFFER) cond++; + err = setvbuf( file_test, NULL, _IONBF, 0 ); + ok( err == 0, "setvbuf failed\n" ); + if (get_buffer_state( file_test ) == BS_CHAR_BUFFER) cond++; + err = setvbuf( file_test, file_test_buffer, _IOFBF, sizeof(file_test_buffer) ); + ok( err == 0, "setvbuf failed\n" ); + if (get_buffer_state( file_test ) == BS_FULL_BUFFER) cond++; + err = setvbuf( file_test, file_test_buffer, _IOLBF, sizeof(file_test_buffer) ); + ok( err == 0, "setvbuf failed\n" ); + if (get_buffer_state( file_test ) == BS_FULL_BUFFER) cond++; + fclose( file_test ); + + if (cond != 4) + { + win_skip("Incompatible Windows platform, skipping test\n"); + trace("DLL2 ucrt=%u msvcrt=%u wine=%u\n", + GetModuleHandleA("ucrtbase.dll") != NULL, + GetModuleHandleA("msvcrt.dll") != NULL, + winetest_platform_is_wine); + winetest_log("trace"); + return FALSE; + } + return TRUE; +} + +/* We could test more types: + * - pipes: + * - console: we assume that NUL and console behave similary (they are of FILE_TYPE_CHAR) + */ +enum handle_creation {H_FILE, H_NUL, H_REOPEN = 0x10, H_KIND_MASK = 0x0F}; + +static void test_std_buffer_state(const char *selfname) +{ + static struct + { + /* input */ + unsigned type; + /* output */ + enum buffer_state std_buffer_state[3]; + int broken_stderr; /* win7,8 and early win10 require it (at least on files) */ + BOOL with_todo; + } + test_file_type[] = + { + /* inheriting from parent */ + /* Note: wine/test.h calls set_vbuf(_IONBF) (even in child) + * hence the result BS_CHAR_BUFFER for stdout without reopen + */ + {H_FILE, {BS_FULL_BUFFER, BS_CHAR_BUFFER, BS_NO_BUFFER}, BS_FULL_BUFFER, TRUE}, + {H_NUL, {BS_FULL_BUFFER, BS_CHAR_BUFFER, BS_NO_BUFFER}, -1}, + /* reopen in child */ + {H_FILE | H_REOPEN, {BS_FULL_BUFFER, BS_FULL_BUFFER, BS_NO_BUFFER}, BS_FULL_BUFFER, TRUE}, + {H_NUL | H_REOPEN, {BS_FULL_BUFFER, BS_NO_BUFFER, BS_NO_BUFFER}, -1}, + }; + char cmdline[MAX_PATH]; + SECURITY_ATTRIBUTES sa = {.nLength = sizeof(sa), .lpSecurityDescriptor = NULL, .bInheritHandle = TRUE}; + STARTUPINFOA startup; + PROCESS_INFORMATION proc; + DWORD exit_code, expected_exit_code, broken_exit_code; + unsigned i; + BOOL ret; + + /* check that get_buffer_state is coherent */ + if (!check_buffer_state_discovery()) return; + + for (i = 0; i < ARRAY_SIZE(test_file_type); i++) + { + winetest_push_context( "stdbuf[%d]", i) ; + memset( &startup, 0, sizeof(startup) ); + startup.cb = sizeof(startup); + + winetest_log("delete"); + + switch ((int)test_file_type[i].type) + { + case H_FILE: + /* so that we can read from stdin */ + create_file( "file0.tmp", "first content\r\n" ); + startup.hStdInput = CreateFileA( "file0.tmp", GENERIC_READ, + FILE_SHARE_READ, &sa, OPEN_EXISTING, 0, NULL ); + startup.hStdOutput = CreateFileA( "file1.tmp", GENERIC_WRITE, + FILE_SHARE_WRITE, &sa, CREATE_ALWAYS, 0, NULL ); + startup.hStdError = CreateFileA( "file2.tmp", GENERIC_WRITE, + FILE_SHARE_WRITE, &sa, CREATE_ALWAYS, 0, NULL ); + startup.dwFlags = STARTF_USESTDHANDLES; + break; + case H_NUL: + startup.hStdInput = CreateFileA( "NUL", GENERIC_READ, + FILE_SHARE_READ, &sa, OPEN_EXISTING, 0, NULL ); + startup.hStdOutput = CreateFileA( "NUL", GENERIC_WRITE, + FILE_SHARE_WRITE, &sa, CREATE_ALWAYS, 0, NULL ); + startup.hStdError = CreateFileA( "NUL", GENERIC_WRITE, + FILE_SHARE_WRITE, &sa, CREATE_ALWAYS, 0, NULL ); + startup.dwFlags = STARTF_USESTDHANDLES; + break; + case H_FILE | H_REOPEN: + create_file( "file0.tmp", "first content\r\n" ); + break; + case H_NUL | H_REOPEN: + break; + default: ok(0, "\n"); break; + } + + sprintf( cmdline, "%s file stdbuf %#x", selfname, test_file_type[i].type ); + ret = CreateProcessA( NULL, cmdline, NULL, NULL, (startup.dwFlags & STARTF_USESTDHANDLES), 0, NULL, NULL, &startup, &proc ); + ok(ret, "Couldn't create child process '%s' %p %p %p: %lu\n", cmdline, startup.hStdInput, startup.hStdOutput, startup.hStdError, GetLastError()); + 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" ); + expected_exit_code = test_file_type[i].std_buffer_state[0] << 0 | + test_file_type[i].std_buffer_state[1] << 2 | + test_file_type[i].std_buffer_state[2] << 4; + if (test_file_type[i].broken_stderr == -1) + broken_exit_code = expected_exit_code; + else + broken_exit_code = test_file_type[i].std_buffer_state[0] << 0 | + test_file_type[i].std_buffer_state[1] << 2 | + test_file_type[i].broken_stderr << 4; + + todo_wine_if( test_file_type[i].with_todo ) + ok( exit_code == expected_exit_code || broken(exit_code == broken_exit_code), + "Unexpected buffer state %lx: stdin=%lx/%x, stdout=%lx/%x, stderr=%lx/%x\n", + exit_code >> 6, + (exit_code >> 0) & 3, test_file_type[i].std_buffer_state[0], + (exit_code >> 2) & 3, test_file_type[i].std_buffer_state[1], + (exit_code >> 4) & 3, test_file_type[i].std_buffer_state[2] ); + + CloseHandle( proc.hProcess ); + CloseHandle( proc.hThread ); + + winetest_log("trace"); + + switch ((int)test_file_type[i].type) + { + case H_FILE: + case H_NUL: + CloseHandle( startup.hStdInput ); + CloseHandle( startup.hStdOutput ); + CloseHandle( startup.hStdError ); + break; + default: + break; + } + DeleteFileA( "file0.tmp" ); + DeleteFileA( "file1.tmp" ); + DeleteFileA( "file2.tmp" ); + winetest_pop_context(); + } +} + +static void test_std_buffer_state_child(int argc, char **argv) +{ + unsigned flags = 0; + unsigned ret = 0; + char ch; + + if (argc >= 4) flags = strtol( argv[3], NULL, 0 ); + + if (flags & H_REOPEN) + { + switch (flags & H_KIND_MASK) + { + case H_FILE: + if (!freopen( "file0.tmp", "r", stdin )) ret |= 1 << 6; + if (!freopen( "file1.tmp", "w", stdout )) ret |= 1 << 6; + if (!freopen( "file2.tmp", "w", stderr )) ret |= 1 << 6; + break; + case H_NUL: + if (!freopen( "NUL", "r", stdin )) ret |= 1 << 6; + if (!freopen( "NUL", "w", stdout )) ret |= 1 << 6; + if (!freopen( "NUL", "w", stderr )) ret |= 1 << 6; + break; + default: + ret |= 1 << 6; + break; + } + } + + /* these operations shall force buffer creation (if required) */ + fscanf( stdin, "%c", &ch ); + fprintf( stdout, "f" ); + fprintf( stderr, "f" ); + + ret |= get_buffer_state( stdin ) << 0; + ret |= get_buffer_state( stdout ) << 2; + ret |= get_buffer_state( stderr ) << 4; + + ExitProcess( ret ); +} + +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 | 6 ++---- 2 files changed, 8 insertions(+), 4 deletions(-) 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 97f2f82ff75..1fad7577be6 100644 --- a/dlls/ucrtbase/tests/file.c +++ b/dlls/ucrtbase/tests/file.c @@ -182,7 +182,6 @@ static void test_std_buffer_state(const char *selfname) /* output */ enum buffer_state std_buffer_state[3]; int broken_stderr; /* win7,8 and early win10 require it (at least on files) */ - BOOL with_todo; } test_file_type[] = { @@ -190,10 +189,10 @@ static void test_std_buffer_state(const char *selfname) /* Note: wine/test.h calls set_vbuf(_IONBF) (even in child) * hence the result BS_CHAR_BUFFER for stdout without reopen */ - {H_FILE, {BS_FULL_BUFFER, BS_CHAR_BUFFER, BS_NO_BUFFER}, BS_FULL_BUFFER, TRUE}, + {H_FILE, {BS_FULL_BUFFER, BS_CHAR_BUFFER, BS_NO_BUFFER}, BS_FULL_BUFFER}, {H_NUL, {BS_FULL_BUFFER, BS_CHAR_BUFFER, BS_NO_BUFFER}, -1}, /* reopen in child */ - {H_FILE | H_REOPEN, {BS_FULL_BUFFER, BS_FULL_BUFFER, BS_NO_BUFFER}, BS_FULL_BUFFER, TRUE}, + {H_FILE | H_REOPEN, {BS_FULL_BUFFER, BS_FULL_BUFFER, BS_NO_BUFFER}, BS_FULL_BUFFER}, {H_NUL | H_REOPEN, {BS_FULL_BUFFER, BS_NO_BUFFER, BS_NO_BUFFER}, -1}, }; char cmdline[MAX_PATH]; @@ -262,7 +261,6 @@ static void test_std_buffer_state(const char *selfname) test_file_type[i].std_buffer_state[1] << 2 | test_file_type[i].broken_stderr << 4; - todo_wine_if( test_file_type[i].with_todo ) ok( exit_code == expected_exit_code || broken(exit_code == broken_exit_code), "Unexpected buffer state %lx: stdin=%lx/%x, stdout=%lx/%x, stderr=%lx/%x\n", exit_code >> 6, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5462
I don't like how the tests are written. It seems to be over complicated while not testing much on Windows (because of the broken case). The tests are also not deleting some files. Did you consider something along these lines to detect if FILE is buffered? ```c struct UCRT_FILE *err = (struct UCRT_FILE *)stderr; int fd = fileno(stderr); ok(fd == err->_file, "unknown FILE layout\n"); err->_file = 0; ok(freopen("test.log", "w", stderr) != NULL, "freopen failed\n"); ok(fprintf(stderr, "out\n") == 4, "fprintf failed\n"); ok(_tell(fileno(stderr)) == 4, "expected file to be unbuffered\n"); fflush(stderr); close(fileno(stderr)); err->_file = fd; ``` It reduces the information we need to know about FILE layout to one field (_file). It's also hacky but looks much simpler for me. Also it would be nice to find a way to detect older versions of ucrtbase so you don't end doing: ```c long pos = _tell(fileno(stderr)); ok(pos == 4 || broken(!pos), ...); ``` Do you know if there's a new function export that can be used to skip the tests on older version of ucrtbase? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5462#note_66977
On Thu Apr 4 19:23:35 2024 +0000, Piotr Caban wrote:
I don't like how the tests are written. It seems to be over complicated while not testing much on Windows (because of the broken case). The tests are also not deleting some files. Did you consider something along these lines to detect if FILE is buffered? ```c struct UCRT_FILE *err = (struct UCRT_FILE *)stderr; int fd = fileno(stderr); ok(fd == err->_file, "unknown FILE layout\n"); err->_file = 0; ok(freopen("test.log", "w", stderr) != NULL, "freopen failed\n"); ok(fprintf(stderr, "out\n") == 4, "fprintf failed\n"); ok(_tell(fileno(stderr)) == 4, "expected file to be unbuffered\n"); fflush(stderr); close(fileno(stderr)); err->_file = fd; ``` It reduces the information we need to know about FILE layout to one field (_file). It's also hacky but looks much simpler for me. Also it would be nice to find a way to detect older versions of ucrtbase so you don't end doing: ```c long pos = _tell(fileno(stderr)); ok(pos == 4 || broken(!pos), ...); ``` Do you know if there's a new function export that can be used to skip the tests on older version of ucrtbase? I think it's even better to test stderr in new process so we don't have to restore it after test. This way we don't need to know FILE layout at all.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5462#note_66979
yes I did (in fact first version of the test was along these lines |A]) but it doesn't work for FILE_TYPE_CHAR or PIPE types we may also want to test that's why I considered another implementation to support more types, and input or output streams perhaps another alternative to not use UCRT FILE layout would be to use _get_stream_buffer_pointers() for the ucrtbase tests and check if the test should work depending just on the returned 3 fields (basically dropping the bufsize related tests, and guessing the charbuf by being inside [file, file+1( memory range (or a fixed length if the MSVCRT FILE is too short wrt/ field offset of charbuf in UCRT FILE)) [A] + could even detect if line buffering is happening + fragile on console output because of scrolling (hence the move to top of screenbuffer) + need more work on PIPE + need extra work on input for console ``` #include <wincon.h> static unsigned get_handle_position(HANDLE h) { CONSOLE_SCREEN_BUFFER_INFO sbi; if (GetConsoleScreenBufferInfo(h, &sbi)) return sbi.dwSize.X * sbi.dwCursorPosition.Y + sbi.dwCursorPosition.X; return GetFileSize(h, NULL); } static enum buffer_state get_buffer_state(FILE *f) { HANDLE handle = (HANDLE)_get_osfhandle(_fileno(f)); unsigned sz_beg, sz_nonl, sz_nl, sz_flush; COORD c = {}; SetConsoleCursorPosition(handle, c); /* to avoid scrolling if connected to console */ sz_beg = get_handle_position(handle); fwrite("foo", 3, 1, f); sz_nonl = get_handle_position(handle); fwrite("bar\n", 4, 1, f); sz_nl = get_handle_position(handle); fflush(f); sz_flush = get_handle_position(handle); ok(sz_beg <= sz_nonl && sz_nonl <= sz_nl && sz_nl <= sz_flush && sz_beg < sz_flush, "SDF\n"); if (sz_beg == sz_nl && sz_nl < sz_flush) return BUFFER_IOFBF; if (sz_beg == sz_nonl && sz_nonl < sz_nl && sz_nl == sz_flush) return BUFFER_IOLBF; if (sz_beg < sz_nonl && sz_nonl < sz_nl && sz_nl == sz_flush) return BUFFER_IONBF; return BUFFER_ERROR; } ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5462#note_67011
* could even detect if line buffering is happening
It can be also detected with what I have proposed.
* fragile on console output because of scrolling (hence the move to top of screenbuffer)
It doesn't really make sense to test console output. It can't be buffered because it will break mixing stdout and stderr.
* need more work on PIPE
If tests on pipes are needed it should be easy to check if data is available on other end. I think you're trying to test to much ending depending on implementation details / object internals. We should focus on things observable by applications. I'm not happy with the change made in native ucrtbase (one can create a bug saying that it's much slower in some scenarios when stderr is unbuffered) but I think we should follow native development. What I want to make sure is to have the tests failing if native decides to restore old behavior. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5462#note_67038
participants (3)
-
Eric Pouech -
eric pouech (＠epo)
-
Piotr Caban (＠piotr)