[PATCH v5 0/1] MR11566: tests: Fix tests that are faulty because of == vs ?: operator precedence.
`==` has higher precedence than `?:`, meaning that a test like `ok(hr == (i < 4) ? S_OK : VFW_E_TYPE_NOT_ACCEPTED, ...)` is parsed as `(hr == (i < 4)) ? S_OK : VFW_E_TYPE_NOT_ACCEPTED` and not the intended `hr == ((i < 4) ? S_OK : VFW_E_TYPE_NOT_ACCEPTED)`. I discovered this when converting `wine/test.h` to use `bool` for conditions (MR coming soon), this generated warnings like: ``` ../dlls/amstream/tests/amstream.c: In function 'test_media_types': ../dlls/amstream/tests/amstream.c:3159:33: warning: '?:' using integer constants in boolean context [-Wint-in-bool-context] 3159 | ok(hr == (i < 4) ? S_OK : VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#lx on ReceiveConnection for subtype %s.\n", hr, ../include/wine/test.h:388:53: note: in definition of macro 'winetest_ok' 388 | #define winetest_ok(cond, ...) (winetest_should_log(cond) ? winetest_ok_(__VA_ARGS__) : winetest_ok_(NULL)) | ^~~~ ../include/wine/test.h:129:18: note: in expansion of macro 'ok_' 129 | #define ok ok_(__FILE__, __LINE__) | ^~~ ../dlls/amstream/tests/amstream.c:3159:9: note: in expansion of macro 'ok' 3159 | ok(hr == (i < 4) ? S_OK : VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#lx on ReceiveConnection for subtype %s.\n", hr, ``` -- v5: tests: Fix tests that are faulty because of == vs ?: operator precedence. https://gitlab.winehq.org/wine/wine/-/merge_requests/11566
From: Brendan Shanks <bshanks@codeweavers.com> --- dlls/amstream/tests/amstream.c | 2 +- dlls/comctl32/tests/pager.c | 2 +- dlls/comctl32/tests/propsheet.c | 4 ++-- dlls/compstui/tests/compstui.c | 2 +- dlls/d3d9/tests/visual.c | 2 +- dlls/dbghelp/tests/dbghelp.c | 2 +- dlls/dxgi/tests/dxgi.c | 2 +- dlls/iphlpapi/tests/iphlpapi.c | 4 ++-- dlls/kernel32/tests/locale.c | 8 ++++---- dlls/kernel32/tests/virtual.c | 2 +- dlls/ntdll/tests/exception.c | 4 ++-- dlls/quartz/tests/avidec.c | 2 +- 12 files changed, 18 insertions(+), 18 deletions(-) diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c index dbff4eb42f9..a9ae9c8e769 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -3156,7 +3156,7 @@ static void test_media_types(void) ok(hr == VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#lx for subtype %s.\n", hr, wine_dbgstr_guid(rejected_subtypes[i].guid)); hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &mt); - ok(hr == (i < 4) ? S_OK : VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#lx on ReceiveConnection for subtype %s.\n", hr, + ok(hr == ((i < 4) ? S_OK : VFW_E_TYPE_NOT_ACCEPTED), "Got hr %#lx on ReceiveConnection for subtype %s.\n", hr, wine_dbgstr_guid(rejected_subtypes[i].guid)); if (hr == S_OK) diff --git a/dlls/comctl32/tests/pager.c b/dlls/comctl32/tests/pager.c index 7f670440dd8..2c41ef6b485 100644 --- a/dlls/comctl32/tests/pager.c +++ b/dlls/comctl32/tests/pager.c @@ -922,7 +922,7 @@ static void send_notify(HWND pager, UINT unicode, UINT ansi, LPARAM lParam, BOOL SendMessageW(pager, WM_NOTIFY, hdr->idFrom, lParam); ok(notify_test_info.received, "Expect notification received\n"); - ok(hdr->code == code_change ? ansi : unicode, "Expect 0x%08x, got 0x%08x\n", hdr->code, + ok(hdr->code == (code_change ? ansi : unicode), "Expect 0x%08x, got 0x%08x\n", hdr->code, code_change ? ansi : unicode); } diff --git a/dlls/comctl32/tests/propsheet.c b/dlls/comctl32/tests/propsheet.c index 812f6530478..028f410f5ee 100644 --- a/dlls/comctl32/tests/propsheet.c +++ b/dlls/comctl32/tests/propsheet.c @@ -1149,7 +1149,7 @@ static void test_CreatePropertySheetPage(void) else { ok(hpsp != NULL, "Failed to create a page, size %lu\n", page.u.pageA.dwSize); - ok(page.addref_called == (page.u.pageA.dwSize > PROPSHEETPAGEA_V1_SIZE) ? 1 : 0, "Expected ADDREF callback message\n"); + ok(page.addref_called == ((page.u.pageA.dwSize > PROPSHEETPAGEA_V1_SIZE) ? 1 : 0), "Expected ADDREF callback message\n"); } if (hpsp) @@ -1192,7 +1192,7 @@ static void test_CreatePropertySheetPage(void) else { ok(hpsp != NULL, "Failed to create a page, size %lu\n", page.u.pageW.dwSize); - ok(page.addref_called == (page.u.pageW.dwSize > PROPSHEETPAGEW_V1_SIZE) ? 1 : 0, "Expected ADDREF callback message\n"); + ok(page.addref_called == ((page.u.pageW.dwSize > PROPSHEETPAGEW_V1_SIZE) ? 1 : 0), "Expected ADDREF callback message\n"); } if (hpsp) diff --git a/dlls/compstui/tests/compstui.c b/dlls/compstui/tests/compstui.c index 74d7e34e39e..111897e0dde 100644 --- a/dlls/compstui/tests/compstui.c +++ b/dlls/compstui/tests/compstui.c @@ -117,7 +117,7 @@ static LONG WINAPI device_property_sheets(PROPSHEETUI_INFO *info, LPARAM lparam) ok(info->cbSize == sizeof(*info), "info->cbSize = %hd\n", info->cbSize); ok(info->Version == PROPSHEETUI_INFO_VERSION, "info->Version = %hd\n", info->Version); - ok(info->Flags == info->lParamInit ? PSUIINFO_UNICODE : 0, "info->Flags = %hd\n", info->Flags); + ok(info->Flags == (info->lParamInit ? PSUIINFO_UNICODE : 0), "info->Flags = %hd\n", info->Flags); ok(info->Reason == PROPSHEETUI_REASON_INIT || info->Reason == PROPSHEETUI_REASON_DESTROY, "info->Reason = %hx\n", info->Reason); ok(info->hComPropSheet != NULL, "info->hComPropSheet = NULL\n"); diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index e070210c6d1..8cb932dd3bb 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -4467,7 +4467,7 @@ static void stretchrect_test(void) hr = IDirect3DDevice9_StretchRect(device, surfaces[test->src], test->src_rect, surfaces[test->dst], test->dst_rect, test->filter); todo_wine_if(test->todo) - ok(hr == test->allowed ? D3D_OK : D3DERR_INVALIDCALL, "Test %u, got unexpected hr %#lx.\n", i, hr); + ok(hr == (test->allowed ? D3D_OK : D3DERR_INVALIDCALL), "Test %u, got unexpected hr %#lx.\n", i, hr); } for (i = 0; i < ARRAY_SIZE(surfaces); ++i) diff --git a/dlls/dbghelp/tests/dbghelp.c b/dlls/dbghelp/tests/dbghelp.c index 9387804a1b2..1f5bdfcf22d 100644 --- a/dlls/dbghelp/tests/dbghelp.c +++ b/dlls/dbghelp/tests/dbghelp.c @@ -683,7 +683,7 @@ static void test_modules_overlap(void) ok(ret, "SymInitialize failed: %lu\n", GetLastError()); base[0] = SymLoadModuleEx(dummy, NULL, target1_dll, NULL, tests[i].first_base, 0, NULL, 0); - ok(base[0] == tests[i].first_base ? tests[i].first_base : base0, "SymLoadModuleEx failed: %lu\n", GetLastError()); + ok(base[0] == (tests[i].first_base ? tests[i].first_base : base0), "SymLoadModuleEx failed: %lu\n", GetLastError()); ret = SymAddSymbol(dummy, base[0], "winetest_symbol_virtual", base[0] + (3 * imsize0) / 4, 13, 0); ok(ret, "SymAddSymbol failed: %lu\n", GetLastError()); diff --git a/dlls/dxgi/tests/dxgi.c b/dlls/dxgi/tests/dxgi.c index 59ed17f00a2..2c562438eca 100644 --- a/dlls/dxgi/tests/dxgi.c +++ b/dlls/dxgi/tests/dxgi.c @@ -8088,7 +8088,7 @@ static void test_colour_space_support(IUnknown *device, BOOL is_d3d12) } hr = IDXGISwapChain3_SetColorSpace1(swapchain3, colour_spaces[i]); - ok(hr == (support & DXGI_SWAP_CHAIN_COLOR_SPACE_SUPPORT_FLAG_PRESENT) ? S_OK : E_INVALIDARG, + ok(hr == ((support & DXGI_SWAP_CHAIN_COLOR_SPACE_SUPPORT_FLAG_PRESENT) ? S_OK : E_INVALIDARG), "Got unexpected hr %#lx for text %u.\n", hr, i); } diff --git a/dlls/iphlpapi/tests/iphlpapi.c b/dlls/iphlpapi/tests/iphlpapi.c index 982880fdb9d..775d734bb02 100644 --- a/dlls/iphlpapi/tests/iphlpapi.c +++ b/dlls/iphlpapi/tests/iphlpapi.c @@ -3459,7 +3459,7 @@ static void test_ParseNetworkString(void) ok(ret == ipv4_address_tests[i].ret, "%s gave error %ld\n", ipv4_address_tests[i].str, ret); - ok(info.Format == ret ? NET_ADDRESS_FORMAT_UNSPECIFIED : NET_ADDRESS_IPV4, + ok(info.Format == (ret ? NET_ADDRESS_FORMAT_UNSPECIFIED : NET_ADDRESS_IPV4), "%s gave format %d\n", ipv4_address_tests[i].str, info.Format); ok(info.Ipv4Address.sin_addr.S_un.S_addr == (ret ? 0x99999999 : ipv4_address_tests[i].addr.S_un.S_addr), "%s gave address %d.%d.%d.%d\n", ipv4_address_tests[i].str, @@ -3485,7 +3485,7 @@ static void test_ParseNetworkString(void) ok(ret == ipv4_service_tests[i].ret, "%s gave error %ld\n", ipv4_service_tests[i].str, ret); - ok(info.Format == ret ? NET_ADDRESS_FORMAT_UNSPECIFIED : NET_ADDRESS_IPV4, + ok(info.Format == (ret ? NET_ADDRESS_FORMAT_UNSPECIFIED : NET_ADDRESS_IPV4), "%s gave format %d\n", ipv4_address_tests[i].str, info.Format); ok(info.Ipv4Address.sin_addr.S_un.S_addr == (ret ? 0x99999999 : ipv4_service_tests[i].addr.S_un.S_addr), "%s gave address %d.%d.%d.%d\n", ipv4_service_tests[i].str, diff --git a/dlls/kernel32/tests/locale.c b/dlls/kernel32/tests/locale.c index d957c2f4019..7d956bca38f 100644 --- a/dlls/kernel32/tests/locale.c +++ b/dlls/kernel32/tests/locale.c @@ -5531,7 +5531,7 @@ static void test_IdnToNameprepUnicode(void) test_data[1].in_len, NULL, 0); err = GetLastError(); ok(ret == test_data[1].ret, "ret = %ld\n", ret); - ok(err == ret ? 0xdeadbeef : ERROR_INVALID_NAME, "err = %ld\n", err); + ok(err == (ret ? 0xdeadbeef : ERROR_INVALID_NAME), "err = %ld\n", err); SetLastError(0xdeadbeef); ret = pIdnToNameprepUnicode(0, test_data[0].in, -1, buf, ARRAY_SIZE(buf)); @@ -5580,7 +5580,7 @@ static void test_IdnToNameprepUnicode(void) if (ret == test_data[i].ret) { - ok(err == ret ? 0xdeadbeef : ERROR_INVALID_NAME, "%ld: err = %ld\n", i, err); + ok(err == (ret ? 0xdeadbeef : ERROR_INVALID_NAME), "%ld: err = %ld\n", i, err); ok(!wcsncmp(test_data[i].out, buf, ret), "%ld: buf = %s\n", i, wine_dbgstr_wn(buf, ret)); } if (pRtlNormalizeString) @@ -5639,7 +5639,7 @@ static void test_IdnToAscii(void) ret = pIdnToAscii(test_data[i].flags, test_data[i].in, test_data[i].in_len, buf, ARRAY_SIZE(buf)); err = GetLastError(); ok(ret == test_data[i].ret || broken(ret == test_data[i].broken_ret), "%ld: ret = %ld\n", i, ret); - ok(err == ret ? 0xdeadbeef : ERROR_INVALID_NAME, "%ld: err = %ld\n", i, err); + ok(err == (ret ? 0xdeadbeef : ERROR_INVALID_NAME), "%ld: err = %ld\n", i, err); ok(!wcsnicmp(test_data[i].out, buf, ret), "%ld: buf = %s\n", i, wine_dbgstr_wn(buf, ret)); } } @@ -5687,7 +5687,7 @@ static void test_IdnToUnicode(void) ret = pIdnToUnicode(test_data[i].flags, test_data[i].in, test_data[i].in_len, buf, ARRAY_SIZE(buf)); err = GetLastError(); ok(ret == test_data[i].ret || broken(ret == test_data[i].broken_ret), "%ld: ret = %ld\n", i, ret); - ok(err == ret ? 0xdeadbeef : ERROR_INVALID_NAME, "%ld: err = %ld\n", i, err); + ok(err == (ret ? 0xdeadbeef : ERROR_INVALID_NAME), "%ld: err = %ld\n", i, err); ok(!wcsncmp(test_data[i].out, buf, ret), "%ld: buf = %s\n", i, wine_dbgstr_wn(buf, ret)); } } diff --git a/dlls/kernel32/tests/virtual.c b/dlls/kernel32/tests/virtual.c index 74b03ec8f54..af740a250ac 100644 --- a/dlls/kernel32/tests/virtual.c +++ b/dlls/kernel32/tests/virtual.c @@ -4200,7 +4200,7 @@ static void test_mapping( HANDLE hfile, DWORD sec_flags, BOOL readonly ) ok(info.AllocationProtect == info.Protect, "%ld: (%04lx) got %#lx, expected %#lx\n", j, view[j].access, info.AllocationProtect, info.Protect); ok(info.State == MEM_COMMIT, "%ld: (%04lx) got %#lx, expected MEM_COMMIT\n", j, view[j].access, info.State); - ok(info.Type == (sec_flags & SEC_IMAGE) ? SEC_IMAGE : MEM_MAPPED, + ok(info.Type == ((sec_flags & SEC_IMAGE) ? SEC_IMAGE : MEM_MAPPED), "%ld: (%04lx) got %#lx, expected MEM_MAPPED\n", j, view[j].access, info.Type); if (nt_base && base) diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index a1758a23f28..8c76661eb6b 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -1594,7 +1594,7 @@ static DWORD simd_fault_handler( EXCEPTION_RECORD *rec, EXCEPTION_REGISTRATION_R ok( rec->ExceptionCode == expected, "exception code: %#lx, should be %#lx\n", rec->ExceptionCode, expected); - ok( rec->NumberParameters == is_wow64 ? 2 : 1, "# of params: %li\n", rec->NumberParameters); + ok( rec->NumberParameters == (is_wow64 ? 2 : 1), "# of params: %li\n", rec->NumberParameters); ok( rec->ExceptionInformation[0] == 0, "param #0: %Ix\n", rec->ExceptionInformation[0]); if (rec->NumberParameters == 2) ok( rec->ExceptionInformation[1] == ((XSAVE_FORMAT *)context->ExtendedRegisters)->MxCsr, @@ -4737,7 +4737,7 @@ static void test_wow64_context(void) context.ContextFlags = CONTEXT_ALL; ret = pNtGetContextThread( pi.hThread, &context ); ok(ret == STATUS_SUCCESS, "got %#lx\n", ret); - ok( context.ContextFlags == is_arm64ec ? CONTEXT_FULL : CONTEXT_ALL, + ok( context.ContextFlags == (is_arm64ec ? CONTEXT_FULL : CONTEXT_ALL), "got context flags %#lx\n", context.ContextFlags ); ok( !context.Rsi, "rsi is not zero %Ix\n", context.Rsi ); ok( !context.Rdi, "rdi is not zero %Ix\n", context.Rdi ); diff --git a/dlls/quartz/tests/avidec.c b/dlls/quartz/tests/avidec.c index 4edb027e5ae..18cb95a50fb 100644 --- a/dlls/quartz/tests/avidec.c +++ b/dlls/quartz/tests/avidec.c @@ -1047,7 +1047,7 @@ static HRESULT WINAPI testsink_Receive(struct strmbase_sink *iface, IMediaSample hr = IMediaSample_GetMediaTime(sample, &start, &stop); ok(hr == VFW_E_MEDIA_TIME_NOT_SET, "Got hr %#lx.\n", hr); hr = IMediaSample_IsDiscontinuity(sample); - todo_wine_if (testmode == 5) ok(hr == (testmode == 4) ? S_OK : S_FALSE, "Got hr %#lx.\n", hr); + todo_wine_if (testmode == 5) ok(hr == ((testmode == 4) ? S_OK : S_FALSE), "Got hr %#lx.\n", hr); hr = IMediaSample_IsPreroll(sample); todo_wine_if (testmode == 3) ok(hr == S_FALSE, "Got hr %#lx.\n", hr); hr = IMediaSample_IsSyncPoint(sample); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11566
On Tue Aug 18 08:14:17 2026 +0000, Dmitry Timoshkov wrote:
It seems that the failures are intermittent. I also see the failures when running the test in a loop. I went ahead and added similar test to dlls/user32/tests.msg.c,ok_sequence_() for actual->message == WM_NCPAINT && actual->wParam != 1, and also see the failures. https://gitlab.winehq.org/wine/wine/-/merge_requests/5777 has fixed a real application and has a comment with an explanation of the bug. Could it be that for a .Net app GDI handles are generated differently or for a .Net app WM_NCPAINT has a truncated wParam? It seems like !5777 should be reverted, but I'm not sure what the correct fix is. Maybe there is something different with GDI handles or WM_NCPAINT in .Net? Or maybe on Windows the generation increments much more slowly so the bug is never triggered? Is the application publicly available?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11566#note_149494
On Fri Aug 21 03:30:26 2026 +0000, Brendan Shanks wrote:
It seems like !5777 should be reverted, but I'm not sure what the correct fix is. Maybe there is something different with GDI handles or WM_NCPAINT in .Net? Or maybe on Windows the generation increments much more slowly so the bug is never triggered? Is the application publicly available? Unfortunately the application is not publicly available. I guess that a test application that reproduces the problem could be created using the pseudo-code I described in https://gitlab.winehq.org/wine/wine/-/merge_requests/5777. While the fix might be not completely correct it should be harmless, and reverting the commit will break the application that it has fixed, and potentially it could break other .Net apps.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11566#note_149497
participants (3)
-
Brendan Shanks -
Brendan Shanks (＠bshanks)
-
Dmitry Timoshkov (＠dmitry)