[PATCH v2 0/2] MR8360: include: Don't evaluate format arguments to ok() unless we need them.
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. -- v2: include: Shut the compiler up about PDNS_RECORD array bounds. https://gitlab.winehq.org/wine/wine/-/merge_requests/8360
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
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
On Thu Jun 19 01:56:35 2025 +0000, Yuxuan Shui wrote:
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? And for some reason build-linux doesn't want to pick up my new commit... Let me try force pushing it again.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8360#note_107157
participants (2)
-
Yuxuan Shui -
Yuxuan Shui (＠yshui)