[PATCH 0/1] MR8360: include: Don't evaluate format arguments to ok() unless we need them.

[Yuxuan Shui (＠yshui)]

Across the codebase there are several test cases where the arguments to ok()
is not safe to evaluate unless the test case fails.

For example:

    ok(i == ARRAY_SIZE(array), "i is too small: %d\n", array[i]);

Only when the test fails (i.e. i < ARRAY_SIZE(array)), is array[i] safe to
evaluate.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58320

* * *

The other option is to fix all such cases in the test suite, and add some documentation to make it clear arguments to `ok()` are _always_ evaluated. I don't know which one is better.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8360

[PATCH 1/1] include: Don't evaluate format arguments to ok() unless we need them.

[Yuxuan Shui]

From: Yuxuan Shui <yshui(a)codeweavers.com>

Across the codebase there are several test cases where the arguments to ok()
is not safe to evaluate unless the test case fails.

For example:

    ok(i == ARRAY_SIZE(array), "i is too small: %d\n", array[i]);

Only when the test fails (i.e. i < ARRAY_SIZE(array)), is array[i] safe to
evaluate.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58320
---
 include/wine/test.h | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/include/wine/test.h b/include/wine/test.h
index 8f3f3bb0d50..6e68db6d57c 100644
--- a/include/wine/test.h
+++ b/include/wine/test.h
@@ -84,6 +84,7 @@ struct winetest_thread_data
     char strings[2000];              /* buffer for debug strings */
     char context[8][128];            /* data to print before messages */
     unsigned int context_count;      /* number of context prefixes */
+    int condition;                   /* current condition */
 };
 
 extern struct winetest_thread_data *winetest_get_thread_data(void);
@@ -364,16 +365,27 @@ static int winetest_vok( int condition, const char *msg, va_list args )
     }
 }
 
-static void winetest_ok( int condition, const char *msg, ... ) __WINE_PRINTF_ATTR(2,3);
-static inline void winetest_ok( int condition, const char *msg, ... )
+static void winetest_ok_( const char *msg, ... ) __WINE_PRINTF_ATTR(1,2);
+static inline void winetest_ok_( const char *msg, ... )
 {
     va_list valist;
 
     va_start(valist, msg);
-    winetest_vok(condition, msg, valist);
+    winetest_vok(winetest_get_thread_data()->condition, msg, valist);
     va_end(valist);
 }
 
+static inline int winetest_should_log( int condition )
+{
+    struct winetest_thread_data *data = winetest_get_thread_data();
+    data->condition = condition;
+
+    /* if test is todo, then we would always log, otherwise we only log if test failed. */
+    return data->todo_level || !condition;
+}
+
+#define winetest_ok(cond, ...) (winetest_should_log(cond) ? winetest_ok_(__VA_ARGS__) : winetest_ok_(NULL))
+
 static void winetest_trace( const char *msg, ... ) __WINE_PRINTF_ATTR(1,2);
 static inline void winetest_trace( const char *msg, ... )
 {
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/8360

[Yuxuan Shui (＠yshui)]

i don't understand those array-bounds warnings from gcc, they seem nonsensical to me.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8360#note_107154

[Yuxuan Shui (＠yshui)]

On Thu Jun 19 01:56:35 2025 +0000, Yuxuan Shui wrote:
> i don't understand those array-bounds warnings from gcc, they seem
> nonsensical to me.
> oh, it's complaining when the list is empty `pLastRR` only points to a `PDNS_RECORD` (`= &rr1->pFirstRR`), which is not as big as a `DNS_RECORDW`? why doesn't it complain before, and how does this MR make it suddenly realize this?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8360#note_107155