[alsa-devel] [PATCH 0/3] Make snd_device_name_hint() reentrant
The following 3 patches ensure that snd_device_name_hint() does not mdify any global variable. They fix spurious error messages that may occur when the function is called from multiple threads simultaneously.
Jerome Forissier (3): Add snd_lib_error_set_local() to install a thread-local error handler. snd_device_name_hint(): do not change the global error handler. snd_device_name_hint(): do not use global snd_config.
configure.in | 10 ++++++++ include/error.h | 6 +++++ src/control/namehint.c | 67 ++++++++++++++++++++++++++++---------------------- src/error.c | 24 +++++++++++++++++- 4 files changed, 76 insertions(+), 31 deletions(-)
This is required so we can make other functions reentrant (such as snd_device_name_hint()). The default error handling function snd_lib_error_default() now checks if a local handler exists, and if so, calls it. Otherwise, the previous behavior is unchanged.
Signed-off-by: Jerome Forissier jerome@taodyne.com --- configure.in | 10 ++++++++++ include/error.h | 6 ++++++ src/error.c | 24 +++++++++++++++++++++++- 3 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/configure.in b/configure.in index 4af917b..d675678 100644 --- a/configure.in +++ b/configure.in @@ -281,6 +281,16 @@ else AC_MSG_RESULT(no) fi
+dnl Check for __thread +AC_MSG_CHECKING([for __thread]) +AC_LINK_IFELSE([AC_LANG_PROGRAM([#if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__)) && ((__GNUC__ < 4) || (__GNUC__ == 4 && __GNUC_MINOR__ < 1) || (__GNUC__ == 4 && __GNUC_MINOR__ == 1 && __GNUC_PATCHLEVEL__ < 2)) +#error gcc has this bug: http://gcc.gnu.org/ml/gcc-bugs/2006-09/msg02275.html +#endif], [static __thread int p = 0])], +[AC_DEFINE(HAVE___THREAD, 1, +Define to 1 if compiler supports __thread) +AC_MSG_RESULT([yes])], +[AC_MSG_RESULT([no])]) + dnl Check for librt AC_MSG_CHECKING(for librt) AC_ARG_WITH(librt, diff --git a/include/error.h b/include/error.h index 6d27083..256cd5f 100644 --- a/include/error.h +++ b/include/error.h @@ -74,5 +74,11 @@ extern int snd_lib_error_set_handler(snd_lib_error_handler_t handler); } #endif
+typedef void (*snd_local_error_handler_t)(const char *file, int line, + const char *func, int err, + const char *fmt, va_list arg); + +snd_local_error_handler_t snd_lib_error_set_local(snd_local_error_handler_t func); + #endif /* __ALSA_ERROR_H */
diff --git a/src/error.c b/src/error.c index 7d5f509..85cea5e 100644 --- a/src/error.c +++ b/src/error.c @@ -60,6 +60,21 @@ const char *snd_strerror(int errnum) return snd_error_codes[errnum]; }
+#ifdef HAVE___THREAD +#define TLS_PFX __thread +#else +#define TLS_PFX /* NOP */ +#endif + +static TLS_PFX snd_local_error_handler_t local_error = NULL; + +snd_local_error_handler_t snd_lib_error_set_local(snd_local_error_handler_t func) +{ + snd_local_error_handler_t old = local_error; + local_error = func; + return old; +} + /** * \brief The default error handler function. * \param file The filename where the error was hit. @@ -69,12 +84,19 @@ const char *snd_strerror(int errnum) * \param fmt The message (including the format characters). * \param ... Optional arguments. * - * Prints the error message including location to \c stderr. + * If a local error function has been installed for the current thread by + * \ref snd_lib_error_set_local, it is called. Otherwise, prints the error + * message including location to \c stderr. */ static void snd_lib_error_default(const char *file, int line, const char *function, int err, const char *fmt, ...) { va_list arg; va_start(arg, fmt); + if (local_error) { + local_error(file, line, function, err, fmt, arg); + va_end(arg); + return; + } fprintf(stderr, "ALSA lib %s:%i:(%s) ", file, line, function); vfprintf(stderr, fmt, arg); if (err)
This is the first step towards making this function reentrant.
Signed-off-by: Jerome Forissier jerome@taodyne.com --- src/control/namehint.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/control/namehint.c b/src/control/namehint.c index 19352be..defc036 100644 --- a/src/control/namehint.c +++ b/src/control/namehint.c @@ -80,7 +80,8 @@ static void zero_handler(const char *file ATTRIBUTE_UNUSED, int line ATTRIBUTE_UNUSED, const char *function ATTRIBUTE_UNUSED, int err ATTRIBUTE_UNUSED, - const char *fmt ATTRIBUTE_UNUSED, ...) + const char *fmt ATTRIBUTE_UNUSED, + va_list arg ATTRIBUTE_UNUSED) { }
@@ -212,7 +213,7 @@ static int try_config(struct hint_list *list, const char *base, const char *name) { - snd_lib_error_handler_t eh; + snd_local_error_handler_t eh; snd_config_t *res = NULL, *cfg, *cfg1, *n; snd_config_iterator_t i, next; char *buf, *buf1 = NULL, *buf2; @@ -239,10 +240,9 @@ static int try_config(struct hint_list *list, sprintf(buf, "%s:CARD=%s", name, snd_ctl_card_info_get_id(list->info)); else strcpy(buf, name); - eh = snd_lib_error; - snd_lib_error_set_handler(&zero_handler); + eh = snd_lib_error_set_local(&zero_handler); err = snd_config_search_definition(snd_config, base, buf, &res); - snd_lib_error_set_handler(eh); + snd_lib_error_set_local(eh); if (err < 0) goto __skip_add; cleanup_res = 1; @@ -337,10 +337,9 @@ static int try_config(struct hint_list *list, goto __ok; /* find, if all parameters have a default, */ /* otherwise filter this definition */ - eh = snd_lib_error; - snd_lib_error_set_handler(&zero_handler); + eh = snd_lib_error_set_local(&zero_handler); err = snd_config_search_alias_hooks(snd_config, base, buf, &res); - snd_lib_error_set_handler(eh); + snd_lib_error_set_local(eh); if (err < 0) goto __cleanup; if (snd_config_search(res, "@args", &cfg) >= 0) {
This commit and its parent make the function reentrant.
Signed-off-by: Jerome Forissier jerome@taodyne.com --- src/control/namehint.c | 52 +++++++++++++++++++++++++++++--------------------- 1 file changed, 30 insertions(+), 22 deletions(-)
diff --git a/src/control/namehint.c b/src/control/namehint.c index defc036..8d5e925 100644 --- a/src/control/namehint.c +++ b/src/control/namehint.c @@ -209,7 +209,8 @@ static char *get_dev_name(struct hint_list *list) #define BUF_SIZE 128 #endif
-static int try_config(struct hint_list *list, +static int try_config(snd_config_t *config, + struct hint_list *list, const char *base, const char *name) { @@ -229,7 +230,7 @@ static int try_config(struct hint_list *list, return -ENOMEM; sprintf(buf, "%s.%s", base, name); /* look for redirection */ - if (snd_config_search(snd_config, buf, &cfg) >= 0 && + if (snd_config_search(config, buf, &cfg) >= 0 && snd_config_get_string(cfg, &str) >= 0 && ((strncmp(base, str, strlen(base)) == 0 && str[strlen(base)] == '.') || strchr(str, '.') == NULL)) @@ -241,7 +242,7 @@ static int try_config(struct hint_list *list, else strcpy(buf, name); eh = snd_lib_error_set_local(&zero_handler); - err = snd_config_search_definition(snd_config, base, buf, &res); + err = snd_config_search_definition(config, base, buf, &res); snd_lib_error_set_local(eh); if (err < 0) goto __skip_add; @@ -338,7 +339,7 @@ static int try_config(struct hint_list *list, /* find, if all parameters have a default, */ /* otherwise filter this definition */ eh = snd_lib_error_set_local(&zero_handler); - err = snd_config_search_alias_hooks(snd_config, base, buf, &res); + err = snd_config_search_alias_hooks(config, base, buf, &res); snd_lib_error_set_local(eh); if (err < 0) goto __cleanup; @@ -405,7 +406,7 @@ static const next_devices_t next_devices[] = { }; #endif
-static int add_card(struct hint_list *list, int card) +static int add_card(snd_config_t *config, struct hint_list *list, int card) { int err, ok; snd_config_t *conf, *n; @@ -417,7 +418,7 @@ static int add_card(struct hint_list *list, int card) snd_ctl_card_info_alloca(&info); list->info = info; - err = snd_config_search(snd_config, list->siface, &conf); + err = snd_config_search(config, list->siface, &conf); if (err < 0) return err; sprintf(ctl_name, "hw:%i", card); @@ -448,7 +449,7 @@ static int add_card(struct hint_list *list, int card) ok = 0; for (device = 0; err >= 0 && device <= max_device; device++) { list->device = device; - err = try_config(list, list->siface, str); + err = try_config(config, list, list->siface, str); if (err < 0) break; ok++; @@ -463,7 +464,7 @@ static int add_card(struct hint_list *list, int card) if (err < 0) { list->card = card; list->device = -1; - err = try_config(list, list->siface, str); + err = try_config(config, list, list->siface, str); } if (err == -ENOMEM) goto __error; @@ -492,14 +493,14 @@ static int get_card_name(struct hint_list *list, int card) return 0; }
-static int add_software_devices(struct hint_list *list) +static int add_software_devices(snd_config_t *config, struct hint_list *list) { int err; snd_config_t *conf, *n; snd_config_iterator_t i, next; const char *str;
- err = snd_config_search(snd_config, list->siface, &conf); + err = snd_config_search(config, list->siface, &conf); if (err < 0) return err; snd_config_for_each(i, next, conf) { @@ -508,7 +509,7 @@ static int add_software_devices(struct hint_list *list) continue; list->card = -1; list->device = -1; - err = try_config(list, list->siface, str); + err = try_config(config, list, list->siface, str); if (err == -ENOMEM) return -ENOMEM; } @@ -546,13 +547,14 @@ int snd_device_name_hint(int card, const char *iface, void ***hints) struct hint_list list; char ehints[24]; const char *str; - snd_config_t *conf; + snd_config_t *conf, *local_config = NULL; + snd_config_update_t *local_config_update = NULL; snd_config_iterator_t i, next; int err;
if (hints == NULL) return -EINVAL; - err = snd_config_update(); + err = snd_config_update_r(&local_config, &local_config_update, NULL); if (err < 0) return err; list.list = NULL; @@ -572,18 +574,21 @@ int snd_device_name_hint(int card, const char *iface, void ***hints) list.iface = SND_CTL_ELEM_IFACE_HWDEP; else if (strcmp(iface, "ctl") == 0) list.iface = SND_CTL_ELEM_IFACE_MIXER; - else - return -EINVAL; + else { + err = -EINVAL; + goto __error; + } + list.show_all = 0; list.cardname = NULL; - if (snd_config_search(snd_config, "defaults.namehint.showall", &conf) >= 0) + if (snd_config_search(local_config, "defaults.namehint.showall", &conf) >= 0) list.show_all = snd_config_get_bool(conf) > 0; if (card >= 0) { err = get_card_name(&list, card); if (err >= 0) - err = add_card(&list, card); + err = add_card(local_config, &list, card); } else { - add_software_devices(&list); + add_software_devices(local_config, &list); err = snd_card_next(&card); if (err < 0) goto __error; @@ -591,7 +596,7 @@ int snd_device_name_hint(int card, const char *iface, void ***hints) err = get_card_name(&list, card); if (err < 0) goto __error; - err = add_card(&list, card); + err = add_card(local_config, &list, card); if (err < 0) goto __error; err = snd_card_next(&card); @@ -600,7 +605,7 @@ int snd_device_name_hint(int card, const char *iface, void ***hints) } } sprintf(ehints, "namehint.%s", list.siface); - err = snd_config_search(snd_config, ehints, &conf); + err = snd_config_search(local_config, ehints, &conf); if (err >= 0) { snd_config_for_each(i, next, conf) { if (snd_config_get_string(snd_config_iterator_entry(i), @@ -617,7 +622,6 @@ int snd_device_name_hint(int card, const char *iface, void ***hints) snd_device_name_free_hint((void **)list.list); if (list.cardname) free(list.cardname); - return err; } else { err = hint_list_add(&list, NULL, NULL); if (err < 0) @@ -626,7 +630,11 @@ int snd_device_name_hint(int card, const char *iface, void ***hints) if (list.cardname) free(list.cardname); } - return 0; + if (local_config) + snd_config_delete(local_config); + if (local_config_update) + snd_config_update_free(local_config_update); + return err; }
/**
participants (1)
-
Jerome Forissier