[PATCH v5 0/2] 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.

--
  v5: include: Don't evaluate format arguments to ok() unless we need them.
      include: Shut the compiler up about PDNS_RECORD array bounds.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8360

[PATCH v5 1/2] include: Shut the compiler up about PDNS_RECORD array bounds.

[Yuxuan Shui]

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

---
 include/windns.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/windns.h b/include/windns.h
index a348ab7c2dc..87e8dad37da 100644
--- a/include/windns.h
+++ b/include/windns.h
@@ -680,7 +680,7 @@ typedef struct _DnsRRSet
 {                                      \
     PDNS_RRSET  _prrset = &(rrset);    \
     PDNS_RECORD _prrnew = (pnewRR);    \
-    _prrset->pLastRR->pNext = _prrnew; \
+    *(PDNS_RECORD *)_prrset->pLastRR = _prrnew; \
     _prrset->pLastRR = _prrnew;        \
 }
 
-- 
GitLab


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

[PATCH v5 2/2] 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