[PATCH 0/3] Make snd_device_name_hint() reentrant

[PATCH 0/3] Make snd_device_name_hint() reentrant

[Jerome Forissier]

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(-)

-- 
1.8.1.2

[PATCH 1/3] Add snd_lib_error_set_local() to install a thread-local error handler.

[Jerome Forissier]

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)
-- 
1.8.1.2

[PATCH 2/3] snd_device_name_hint(): do not change the global error handler.

[Jerome Forissier]

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) {
-- 
1.8.1.2

[PATCH 3/3] snd_device_name_hint(): do not use global snd_config.

[Jerome Forissier]

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;
 }
 
 /**
-- 
1.8.1.2