From cba819d65e890ca1db9e9c81f5e44125711d78a8 Mon Sep 17 00:00:00 2001 From: Yonggang Luo Date: Sun, 6 Nov 2022 04:32:05 +0800 Subject: [PATCH] util: Fixes thread safety of DEBUG_GET_ONCE_*_OPTION macros MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pick DEBUG_GET_ONCE_BOOL_OPTION as a example: The intention of DEBUG_GET_ONCE_BOOL_OPTION are returned the same value across thread, before this commit, on different thread call the function generated by DEBUG_GET_ONCE_BOOL_OPTION may return different value if called setenv in the middle of debug_get_bool_option, so use debug_get_option_cached along with new exposed function debug_parse_bool_option to solve this issue Signed-off-by: Yonggang Luo Acked-by: Marek Olšák Part-of: --- src/util/u_debug.c | 42 ++++++++++++++++++++++++++++-------------- src/util/u_debug.h | 27 +++++++++++++++++++++++---- 2 files changed, 51 insertions(+), 18 deletions(-) diff --git a/src/util/u_debug.c b/src/util/u_debug.c index 07c02b572f0..a5f85120a98 100644 --- a/src/util/u_debug.c +++ b/src/util/u_debug.c @@ -100,10 +100,9 @@ debug_disable_win32_error_dialogs(void) } #endif /* _WIN32 */ -static bool -debug_get_bool_option_direct(const char *name, bool dfault) +bool +debug_parse_bool_option(const char *str, bool dfault) { - const char *str = os_get_option(name); bool result; if (str == NULL) @@ -140,7 +139,8 @@ debug_get_option_should_print(void) static bool value = false; if (unlikely(!p_atomic_read_relaxed(&initialized))) { - value = debug_get_bool_option_direct("GALLIUM_PRINT_OPTIONS", false); + bool parsed_value = debug_parse_bool_option(os_get_option("GALLIUM_PRINT_OPTIONS"), false); + p_atomic_set(&value, parsed_value); p_atomic_set(&initialized, true); } @@ -192,7 +192,7 @@ debug_get_option_cached(const char *name, const char *dfault) bool debug_get_bool_option(const char *name, bool dfault) { - bool result = debug_get_bool_option_direct(name, dfault); + bool result = debug_parse_bool_option(os_get_option(name), dfault); if (debug_get_option_should_print()) debug_printf("%s: %s = %s\n", __func__, name, result ? "TRUE" : "FALSE"); @@ -202,12 +202,9 @@ debug_get_bool_option(const char *name, bool dfault) int64_t -debug_get_num_option(const char *name, int64_t dfault) +debug_parse_num_option(const char *str, int64_t dfault) { int64_t result; - const char *str; - - str = os_get_option(name); if (!str) { result = dfault; } else { @@ -219,6 +216,13 @@ debug_get_num_option(const char *name, int64_t dfault) result = dfault; } } + return result; +} + +int64_t +debug_get_num_option(const char *name, int64_t dfault) +{ + int64_t result = debug_parse_num_option(os_get_option(name), dfault); if (debug_get_option_should_print()) debug_printf("%s: %s = %"PRId64"\n", __func__, name, result); @@ -297,16 +301,15 @@ str_has_option(const char *str, const char *name) uint64_t -debug_get_flags_option(const char *name, - const struct debug_named_value *flags, - uint64_t dfault) +debug_parse_flags_option(const char *name, + const char *str, + const struct debug_named_value *flags, + uint64_t dfault) { uint64_t result; - const char *str; const struct debug_named_value *orig = flags; unsigned namealign = 0; - str = os_get_option(name); if (!str) result = dfault; else if (!strcmp(str, "help")) { @@ -328,6 +331,17 @@ debug_get_flags_option(const char *name, } } + return result; +} + +uint64_t +debug_get_flags_option(const char *name, + const struct debug_named_value *flags, + uint64_t dfault) +{ + const char *str = os_get_option(name); + uint64_t result = debug_parse_flags_option(name, str, flags, dfault); + if (debug_get_option_should_print()) { if (str) { debug_printf("%s: %s = 0x%"PRIx64" (%s)\n", diff --git a/src/util/u_debug.h b/src/util/u_debug.h index 61cee1c43bd..4385be93a5d 100644 --- a/src/util/u_debug.h +++ b/src/util/u_debug.h @@ -358,12 +358,24 @@ debug_get_option(const char *name, const char *dfault); const char * debug_get_option_cached(const char *name, const char *dfault); +bool +debug_parse_bool_option(const char *str, bool dfault); + bool debug_get_bool_option(const char *name, bool dfault); +int64_t +debug_parse_num_option(const char *str, int64_t dfault); + int64_t debug_get_num_option(const char *name, int64_t dfault); +uint64_t +debug_parse_flags_option(const char *name, + const char *str, + const struct debug_named_value *flags, + uint64_t dfault); + uint64_t debug_get_flags_option(const char *name, const struct debug_named_value *flags, @@ -376,7 +388,8 @@ debug_get_option_ ## suffix (void) \ static bool initialized = false; \ static const char * value; \ if (unlikely(!p_atomic_read_relaxed(&initialized))) { \ - value = debug_get_option(name, dfault); \ + const char *str = debug_get_option_cached(name, dfault); \ + p_atomic_set(&value, str); \ p_atomic_set(&initialized, true); \ } \ return value; \ @@ -399,7 +412,9 @@ debug_get_option_ ## sufix (void) \ static bool initialized = false; \ static bool value; \ if (unlikely(!p_atomic_read_relaxed(&initialized))) { \ - value = debug_get_bool_option(name, dfault); \ + const char *str = debug_get_option_cached(name, NULL); \ + bool parsed_value = debug_parse_bool_option(str, dfault); \ + p_atomic_set(&value, parsed_value); \ p_atomic_set(&initialized, true); \ } \ return value; \ @@ -412,7 +427,9 @@ debug_get_option_ ## sufix (void) \ static bool initialized = false; \ static int64_t value; \ if (unlikely(!p_atomic_read_relaxed(&initialized))) { \ - value = debug_get_num_option(name, dfault); \ + const char *str = debug_get_option_cached(name, NULL); \ + int64_t parsed_value = debug_parse_num_option(str, dfault); \ + p_atomic_set(&value, parsed_value); \ p_atomic_set(&initialized, true); \ } \ return value; \ @@ -425,7 +442,9 @@ debug_get_option_ ## sufix (void) \ static bool initialized = false; \ static uint64_t value; \ if (unlikely(!p_atomic_read_relaxed(&initialized))) { \ - value = debug_get_flags_option(name, flags, dfault); \ + const char *str = debug_get_option_cached(name, NULL); \ + uint64_t parsed_value = debug_parse_flags_option(name, str, flags, dfault); \ + p_atomic_set(&value, parsed_value); \ p_atomic_set(&initialized, true); \ } \ return value; \