Skip to contentSkip to conversation

topology: Support widgets' stream name and file inclusion for text conf file

PATCH Superseded [email protected] 2 participants 6 messages Oct 20, 2016 – Oct 24, 2016

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v2 cover topology: Support widgets' stream name and file inclusion for text conf file

From: Mengdong Lin <[email protected]>

This series doesn't require or cause any ABI updates. It doesn't need the kernel patches that trying to udpate ABI from v4 or v5.

Patch 1 fixes a missing topology feature for text conf file to configure
stream name of a widget.

Patch 2 allows alsaconf to search included files under global
configuration directories. This will benefit topology and UCM.

Patch 3 adds document about how to include files for topology
configuration. There is no c code change but actually it depends on patch 2.

History:
v2: No longer use topology-specific code to support searching included
files, but extend alsaconf to support this and user can just use alsaconf syntax <xxx> and <confdir:xxx>.

Mengdong Lin (3):
topology: Fix missing stream name of widgets in text conf file
alsaconf: Search included files under global configuration directories
topology: Add doc for including other files in the text conf file

 include/topology.h  |  31 ++++++++++++
 src/conf.c          | 141 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 src/topology/dapm.c |  10 ++++
 3 files changed, 180 insertions(+), 2 deletions(-)
Signature · 1 line
2.5.0
#1 lore raw

PATCH v2 1/3 topology: Fix missing stream name of widgets in text conf file

From: Mengdong Lin <[email protected]>

User can define the stream name of an input/output widget in the text conf file, by setting "stream_name" of a SectionWidget.

Topology C API and kernel already have support for configuring a widget's stream name. This patch just adds the missing part of the text conf file.

include/topology.h +1 -0
index 0675b52..a0d018e 100644
@@ -483,6 +483,7 @@ extern "C" {
  *	index "1"			# Index number
  *
  *	type "aif_in"			# Widget type - detailed above
+ *	stream_name "name"		# Stream name
  *
  *	no_pm "true"			# No PM control bit.
  *	reg "20"			# PM bit register offset
src/topology/dapm.c +10 -0
index e3c90d8..9fa0aac 100644
@@ -507,6 +507,16 @@ int tplg_parse_dapm_widget(snd_tplg_t *tplg,
 			continue;
 		}
 
+		if (strcmp(id, "stream_name") == 0) {
+			if (snd_config_get_string(n, &val) < 0)
+				return -EINVAL;
+
+			elem_copy_text(widget->sname, val,
+				       SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
+			tplg_dbg("\t%s: %s\n", id, val);
+			continue;
+		}
+
 		if (strcmp(id, "no_pm") == 0) {
 			if (snd_config_get_string(n, &val) < 0)
 				return -EINVAL;
Signature · 1 line
2.5.0
#2 lore raw

PATCH v2 2/3 alsaconf: Search included files under global configuration directories

In reply to [email protected]

From: Mengdong Lin <[email protected]>

Users can include file by giving a relative path or just a file name via alsaconf syntax <xxx>. ALSA conf will search the file in top configuration directory and additional configuration directories defined by users via alsaconf syntax <confdir:/xxx>.

ALSA conf will search and open an included file in the following order of priority:

1. directly open the file by its name;
2. search for the file name in top config direcotry "usr/share/alsa/";
3. search for the file name in additional configuration directories specified by users, via alsaconf syntax <confdir:/relative-path/to/user/share/alsa>;

The configuration directories defined by a file will only be used to search the files included by this file.

src/conf.c +139 -2
index a516611..44ef673 100644
@@ -456,6 +456,18 @@ struct filedesc {
 	snd_input_t *in;
 	unsigned int line, column;
 	struct filedesc *next;
+
+	/* list of the include paths (configuration directories),
+	 * defined by <confdir:/relative-path/to/top-alsa-conf-dir>,
+	 * for searching its included files.
+	 */
+	struct list_head include_paths;
+};
+
+/* path to search included files */
+struct include_path {
+	char *dir;
+	struct list_head list;
 };
 
 #define LOCAL_ERROR			(-0x68000000)
@@ -501,6 +513,120 @@ static inline void snd_config_unlock(void) { }
 
 #endif
 
+/**
+ * \brief Add a diretory to the paths to search included files.
+ * \param fd File object that owns these paths to search files included by it.
+ * \param dir Path of the directory to add.
+ * \return Zero if successful, otherwise a negative error code.
+ *
+ * The direcotry should be a subdiretory of top configuration directory
+ * "/usr/share/alsa/".
+ */
+static int add_include_path(struct filedesc *fd, char *dir)
+{
+	struct include_path *path;
+
+	path = calloc(1, sizeof(*path));
+	if (!path)
+		return -ENOMEM;
+
+	path->dir = calloc(1, PATH_MAX + 1);
+	if (!path->dir)
+		return -ENOMEM;
+
+	strncpy(path->dir, dir, PATH_MAX);
+	list_add_tail(&path->list, &fd->include_paths);
+	return 0;
+}
+
+/**
+ * \brief Free all include paths of a file descriptor.
+ * \param fd File object that owns these paths to search files included by it.
+ */
+static void free_include_paths(struct filedesc *fd)
+{
+	struct list_head *pos, *npos, *base;
+	struct include_path *path;
+
+	base = &fd->include_paths;
+	list_for_each_safe(pos, npos, base) {
+		path = list_entry(pos, struct include_path, list);
+		list_del(&path->list);
+		if (path->dir)
+			free(path->dir);
+		free(path);
+	}
+}
+
+/**
+ * \brief Search and open a file, and creates a new input object reading
+ *        from the file.
+ * \param inputp The functions puts the pointer to the new input object
+ *               at the address specified by \p inputp.
+ * \param file Name of the configuration file.
+ * \param include_paths Optional, addtional directories to search the file.
+ * \return Zero if successful, otherwise a negative error code.
+ *
+ * This function will search and open the file in the following order
+ * of priority:
+ * 1. directly open the file by its name;
+ * 2. search for the file name in top configuration directory
+ *     "usr/share/alsa/";
+ * 3. search for the file name in in additional configuration directories
+ *     specified by users, via alsaconf syntax
+ *     <confdir:/relative-path/to/user/share/alsa>;
+ *     These directories should be subdirectories of /usr/share/alsa.
+ */
+static int input_stdio_open(snd_input_t **inputp, const char *file,
+			    struct list_head *include_paths)
+{
+	struct list_head *pos, *npos, *base;
+	struct include_path *path;
+	char *full_path = NULL;
+	int err = 0;
+
+	err = snd_input_stdio_open(inputp, file, "r");
+	if (err == 0)
+		goto out;
+
+	full_path = calloc(1, PATH_MAX + 1);
+	if (!full_path)
+		return -ENOMEM;
+
+	/* search file in top configuration directory usr/share/alsa */
+	strcpy(full_path, ALSA_CONFIG_DIR);
+	strcat(full_path, "/");
+	strcat(full_path, file);
+	err = snd_input_stdio_open(inputp, full_path, "r");
+	if (err == 0)
+		goto out;
+
+	/* search file in user specified include paths. These directories
+	 * are subdirectories of /usr/share/alsa.
+	 */
+	if (include_paths) {
+		base = include_paths;
+		list_for_each_safe(pos, npos, base) {
+			path = list_entry(pos, struct include_path, list);
+			if (!path->dir)
+				continue;
+
+			strncpy(full_path, path->dir, PATH_MAX);
+			strcat(full_path, "/");
+			strcat(full_path, file);
+			err = snd_input_stdio_open(inputp, full_path, "r");
+			if (err == 0)
+			goto out;
+		}
+	}
+
+out:
+	if (full_path)
+		free(full_path);
+
+	return err;
+}
+
 static int safe_strtoll(const char *str, long long *val)
 {
 	long long v;
@@ -632,7 +758,7 @@ static int get_char_skip_comments(input_t *input)
 			int err = get_delimstring(&str, '>', input);
 			if (err < 0)
 				return err;
-			if (!strncmp(str, "confdir:", 8)) {
+			if (!strncmp(str, "confdir:", 8)) { /* directory */
 				char *tmp = malloc(strlen(ALSA_CONFIG_DIR) + 1 + strlen(str + 8) + 1);
 				if (tmp == NULL) {
 					free(str);
@@ -641,8 +767,14 @@ static int get_char_skip_comments(input_t *input)
 				sprintf(tmp, ALSA_CONFIG_DIR "/%s", str + 8);
 				free(str);
 				str = tmp;
+
+				err = snd_input_stdio_open(&in, str, "r");
+				if (err == 0)
+					add_include_path(input->current, str);
+			} else { /* file, also search in include paths */
+				err = input_stdio_open(&in, str,
+						&input->current->include_paths);
 			}
-			err = snd_input_stdio_open(&in, str, "r");
 			if (err < 0) {
 				SNDERR("Cannot access file %s", str);
 				free(str);
@@ -658,6 +790,7 @@ static int get_char_skip_comments(input_t *input)
 			fd->next = input->current;
 			fd->line = 1;
 			fd->column = 0;
+			INIT_LIST_HEAD(&fd->include_paths);
 			input->current = fd;
 			continue;
 		}
@@ -1668,6 +1801,7 @@ static int snd_config_load1(snd_config_t *config, snd_input_t *in, int override)
 	fd->line = 1;
 	fd->column = 0;
 	fd->next = NULL;
+	INIT_LIST_HEAD(&fd->include_paths);
 	input.current = fd;
 	input.unget = 0;
 	err = parse_defs(config, &input, 0, override);
@@ -1708,9 +1842,12 @@ static int snd_config_load1(snd_config_t *config, snd_input_t *in, int override)
 		fd_next = fd->next;
 		snd_input_close(fd->in);
 		free(fd->name);
+		free_include_paths(fd);
 		free(fd);
 		fd = fd_next;
 	}
+
+	free_include_paths(fd);
 	free(fd);
 	return err;
 }
Signature · 1 line
2.5.0
#3 lore raw

Re: [PATCH v2 2/3] alsaconf: Search included files under global configuration directories

Quoted 8 lines · On Thu, 20 Oct 2016 07:48:37 +0200, [email protected] wrote:

From: Mengdong Lin <[email protected]>

Users can include file by giving a relative path or just a file name via alsaconf syntax <xxx>. ALSA conf will search the file in top configuration directory and additional configuration directories defined by users via alsaconf syntax <confdir:/xxx>.

Quoted 8 lines

ALSA conf will search and open an included file in the following order of priority:

1. directly open the file by its name;
2. search for the file name in top config direcotry "usr/share/alsa/";
3. search for the file name in additional configuration directories specified by users, via alsaconf syntax <confdir:/relative-path/to/user/share/alsa>;

This would break the existing use case of <confdir:xxx>. Currently, "confdir:" simply replaces itself with the default config directory. For example, <confdir:foo/bar> will include the file /usr/share/alsa/foo/bar, and it doesn't mean to specify the directory. You can think of "confdir:" as $ALSA_CONF_DIR.

In your use case, you want to enumerate directories. Then either create a new macro (e.g. "searchdir") or check properly whether the given path is a directory.

Quoted 30 lines

The configuration directories defined by a file will only be used to search the files included by this file.

src/conf.c +12 -0
index a516611..44ef673 100644
@@ -456,6 +456,18 @@ struct filedesc {
 	snd_input_t *in;
 	unsigned int line, column;
 	struct filedesc *next;
+
+	/* list of the include paths (configuration directories),
+	 * defined by <confdir:/relative-path/to/top-alsa-conf-dir>,
+	 * for searching its included files.
+	 */
+	struct list_head include_paths;
+};
+
+/* path to search included files */
+struct include_path {
+	char *dir;
+	struct list_head list;
 };
 
 #define LOCAL_ERROR			(-0x68000000)
@@ -501,6 +513,120 @@ static inline void snd_config_unlock(void) { }
 

src/conf.c static inline void snd_config_unlock(void) { } · line 523

Quoted 10 lines
src/conf.c +8 -0
 #endif
 
+/**
+ * \brief Add a diretory to the paths to search included files.
+ * \param fd File object that owns these paths to search files included by it.
+ * \param dir Path of the directory to add.
+ * \return Zero if successful, otherwise a negative error code.
+ *
+ * The direcotry should be a subdiretory of top configuration directory
+ * "/usr/share/alsa/".

Don't use the doxygen comment for static functions. They shouldn't be included in the exported documents.

src/conf.c static inline void snd_config_unlock(void) { } · line 535

Quoted 12 lines
src/conf.c +12 -0
+ */
+static int add_include_path(struct filedesc *fd, char *dir)
+{
+	struct include_path *path;
+
+	path = calloc(1, sizeof(*path));
+	if (!path)
+		return -ENOMEM;
+
+	path->dir = calloc(1, PATH_MAX + 1);
+	if (!path->dir)
+		return -ENOMEM;

Leaking the path object here.

src/conf.c static inline void snd_config_unlock(void) { } · line 537

Quoted 1 line
src/conf.c +1 -0
+	strncpy(path->dir, dir, PATH_MAX);

Isn't the dir the proper NUL-terminated string? If so, we can simply copy like:

	path->dir = strdup(dir);
	if (!path->dir) {
		...

src/conf.c static inline void snd_config_unlock(void) { } · line 545

Quoted 8 lines
src/conf.c +8 -0
+	list_add_tail(&path->list, &fd->include_paths);
+	return 0;
+}
+
+/**
+ * \brief Free all include paths of a file descriptor.
+ * \param fd File object that owns these paths to search files included by it.
+ */

Ditto, no doxygen comments.

Quoted 24 lines
src/conf.c +24 -0
+static void free_include_paths(struct filedesc *fd)
+{
+	struct list_head *pos, *npos, *base;
+	struct include_path *path;
+
+	base = &fd->include_paths;
+	list_for_each_safe(pos, npos, base) {
+		path = list_entry(pos, struct include_path, list);
+		list_del(&path->list);
+		if (path->dir)
+			free(path->dir);
+		free(path);
+	}
+}
+
+/**
+ * \brief Search and open a file, and creates a new input object reading
+ *        from the file.
+ * \param inputp The functions puts the pointer to the new input object
+ *               at the address specified by \p inputp.
+ * \param file Name of the configuration file.
+ * \param include_paths Optional, addtional directories to search the file.
+ * \return Zero if successful, otherwise a negative error code.
+ *

src/conf.c static inline void snd_config_unlock(void) { } · line 579

Quoted 10 lines
src/conf.c +10 -0
+ * This function will search and open the file in the following order
+ * of priority:
+ * 1. directly open the file by its name;
+ * 2. search for the file name in top configuration directory
+ *     "usr/share/alsa/";
+ * 3. search for the file name in in additional configuration directories
+ *     specified by users, via alsaconf syntax
+ *     <confdir:/relative-path/to/user/share/alsa>;
+ *     These directories should be subdirectories of /usr/share/alsa.
+ */

Ditto.

Quoted 5 lines
src/conf.c +5 -0
+static int input_stdio_open(snd_input_t **inputp, const char *file,
+			    struct list_head *include_paths)
+{
+	struct list_head *pos, *npos, *base;
+	struct include_path *path;

src/conf.c static inline void snd_config_unlock(void) { } · line 594

Quoted 10 lines
src/conf.c +10 -0
+	char *full_path = NULL;
+	int err = 0;
+
+	err = snd_input_stdio_open(inputp, file, "r");
+	if (err == 0)
+		goto out;
+
+	full_path = calloc(1, PATH_MAX + 1);
+	if (!full_path)
+		return -ENOMEM;

In user-space we have a bigger stack, so you can just use the auto variable here instead of heap. It's more efficient.

src/conf.c static inline void snd_config_unlock(void) { } · line 599

Quoted 5 lines
src/conf.c +5 -0
+
+	/* search file in top configuration directory usr/share/alsa */
+	strcpy(full_path, ALSA_CONFIG_DIR);
+	strcat(full_path, "/");
+	strcat(full_path, file);

This may overflow. Do like

	snprintf(full_path, sizeof(full_path), "%s/%s",
		ALSA_CONFIG_DIR, file);

src/conf.c static inline void snd_config_unlock(void) { } · line 609

Quoted 10 lines
src/conf.c +10 -0
+	err = snd_input_stdio_open(inputp, full_path, "r");
+	if (err == 0)
+		goto out;
+
+	/* search file in user specified include paths. These directories
+	 * are subdirectories of /usr/share/alsa.
+	 */
+	if (include_paths) {
+		base = include_paths;
+		list_for_each_safe(pos, npos, base) {

No need to use *_safe version here, as we don't remove the list entry.

Quoted 33 lines
src/conf.c +21 -1
+			path = list_entry(pos, struct include_path, list);
+			if (!path->dir)
+				continue;
+
+			strncpy(full_path, path->dir, PATH_MAX);
+			strcat(full_path, "/");
+			strcat(full_path, file);
+			err = snd_input_stdio_open(inputp, full_path, "r");
+			if (err == 0)
+			goto out;
+		}
+	}
+
+out:
+	if (full_path)
+		free(full_path);
+
+	return err;
+}
+
 static int safe_strtoll(const char *str, long long *val)
 {
 	long long v;
@@ -632,7 +758,7 @@ static int get_char_skip_comments(input_t *input)
 			int err = get_delimstring(&str, '>', input);
 			if (err < 0)
 				return err;
-			if (!strncmp(str, "confdir:", 8)) {
+			if (!strncmp(str, "confdir:", 8)) { /* directory */
 				char *tmp = malloc(strlen(ALSA_CONFIG_DIR) + 1 + strlen(str + 8) + 1);
 				if (tmp == NULL) {
 					free(str);

src/conf.c static int get_char_skip_comments(input_t *input) · line 773

Quoted 7 lines
src/conf.c +4 -0
@@ -641,8 +767,14 @@ static int get_char_skip_comments(input_t *input)
 				sprintf(tmp, ALSA_CONFIG_DIR "/%s", str + 8);
 				free(str);
 				str = tmp;
+
+				err = snd_input_stdio_open(&in, str, "r");
+				if (err == 0)
+					add_include_path(input->current, str);

See my comment above.

thanks,

Takashi

Quoted 40 lines
src/conf.c +8 -1
+			} else { /* file, also search in include paths */
+				err = input_stdio_open(&in, str,
+						&input->current->include_paths);
 			}
-			err = snd_input_stdio_open(&in, str, "r");
 			if (err < 0) {
 				SNDERR("Cannot access file %s", str);
 				free(str);
@@ -658,6 +790,7 @@ static int get_char_skip_comments(input_t *input)
 			fd->next = input->current;
 			fd->line = 1;
 			fd->column = 0;
+			INIT_LIST_HEAD(&fd->include_paths);
 			input->current = fd;
 			continue;
 		}
@@ -1668,6 +1801,7 @@ static int snd_config_load1(snd_config_t *config, snd_input_t *in, int override)
 	fd->line = 1;
 	fd->column = 0;
 	fd->next = NULL;
+	INIT_LIST_HEAD(&fd->include_paths);
 	input.current = fd;
 	input.unget = 0;
 	err = parse_defs(config, &input, 0, override);
@@ -1708,9 +1842,12 @@ static int snd_config_load1(snd_config_t *config, snd_input_t *in, int override)
 		fd_next = fd->next;
 		snd_input_close(fd->in);
 		free(fd->name);
+		free_include_paths(fd);
 		free(fd);
 		fd = fd_next;
 	}
+
+	free_include_paths(fd);
 	free(fd);
 	return err;
 }
Signature · 1 line
2.5.0
#4 lore raw

Re: [PATCH v2 2/3] alsaconf: Search included files under global configuration directories

Quoted 25 lines · Takashi Iwai
Quoted 8 lines · On Thu, 20 Oct 2016 07:48:37 +0200, [email protected] wrote:

From: Mengdong Lin <[email protected]>

Users can include file by giving a relative path or just a file name via alsaconf syntax <xxx>. ALSA conf will search the file in top configuration directory and additional configuration directories defined by users via alsaconf syntax <confdir:/xxx>.

Quoted 8 lines

ALSA conf will search and open an included file in the following order of priority:

1. directly open the file by its name;
2. search for the file name in top config direcotry "usr/share/alsa/";
3. search for the file name in additional configuration directories specified by users, via alsaconf syntax <confdir:/relative-path/to/user/share/alsa>;

This would break the existing use case of <confdir:xxx>. Currently, "confdir:" simply replaces itself with the default config directory. For example, <confdir:foo/bar> will include the file /usr/share/alsa/foo/bar, and it doesn't mean to specify the directory. You can think of "confdir:" as $ALSA_CONF_DIR.

Quoted 3 lines

In your use case, you want to enumerate directories. Then either create a new macro (e.g. "searchdir") or check properly whether the given path is a directory.

Thanks for pointing out the mistake.
I'll use <searchdir:xxx> and only files with a relative path
will be searched under <usr/share/alsa> and user specified directories via syntax <searchdir:xxx>.

Quoted 34 lines
Quoted 34 lines

The configuration directories defined by a file will only be used to search the files included by this file.

src/conf.c +12 -0
index a516611..44ef673 100644
@@ -456,6 +456,18 @@ struct filedesc {
  	snd_input_t *in;
  	unsigned int line, column;
  	struct filedesc *next;
+
+	/* list of the include paths (configuration directories),
+	 * defined by <confdir:/relative-path/to/top-alsa-conf-dir>,
+	 * for searching its included files.
+	 */
+	struct list_head include_paths;
+};
+
+/* path to search included files */
+struct include_path {
+	char *dir;
+	struct list_head list;
  };
 
  #define LOCAL_ERROR			(-0x68000000)
@@ -501,6 +513,120 @@ static inline void snd_config_unlock(void) { }
 
  #endif
 
Quoted 13 lines

src/conf.c static inline void snd_config_unlock(void) { } · line 523

Quoted 8 lines
src/conf.c +8 -0
+/**
+ * \brief Add a diretory to the paths to search included files.
+ * \param fd File object that owns these paths to search files included by it.
+ * \param dir Path of the directory to add.
+ * \return Zero if successful, otherwise a negative error code.
+ *
+ * The direcotry should be a subdiretory of top configuration directory
+ * "/usr/share/alsa/".

Don't use the doxygen comment for static functions. They shouldn't be included in the exported documents.

Okay. I'll change the format.

Quoted 6 lines
Quoted 6 lines
src/conf.c +6 -0
+ */
+static int add_include_path(struct filedesc *fd, char *dir)
+{
+	struct include_path *path;
+
+	path = calloc(1, sizeof(*path));
Quoted 9 lines

src/conf.c static inline void snd_config_unlock(void) { } · line 535

Quoted 6 lines
src/conf.c +6 -0
+	if (!path)
+		return -ENOMEM;
+
+	path->dir = calloc(1, PATH_MAX + 1);
+	if (!path->dir)
+		return -ENOMEM;

Leaking the path object here.

Sorry, I'll fix this. New code will have dir allocated by the caller, so here we'll just store the string pointer and there will be no calloc and free on error here.

Quoted 8 lines

src/conf.c static inline void snd_config_unlock(void) { } · line 537

Quoted 1 line
src/conf.c +1 -0
+	strncpy(path->dir, dir, PATH_MAX);

Isn't the dir the proper NUL-terminated string? If so, we can simply copy like:

	path->dir = strdup(dir);
	if (!path->dir) {
		...

Yes.

Quoted 10 lines

src/conf.c static inline void snd_config_unlock(void) { } · line 545

Quoted 8 lines
src/conf.c +8 -0
+	list_add_tail(&path->list, &fd->include_paths);
+	return 0;
+}
+
+/**
+ * \brief Free all include paths of a file descriptor.
+ * \param fd File object that owns these paths to search files included by it.
+ */

Ditto, no doxygen comments.

Okay.

Quoted 29 lines
Quoted 29 lines
src/conf.c +28 -0
+static void free_include_paths(struct filedesc *fd)
+{
+	struct list_head *pos, *npos, *base;
+	struct include_path *path;
+
+	base = &fd->include_paths;
+	list_for_each_safe(pos, npos, base) {
+		path = list_entry(pos, struct include_path, list);
+		list_del(&path->list);
+		if (path->dir)
+			free(path->dir);
+		free(path);
+	}
+}
+
+/**
+ * \brief Search and open a file, and creates a new input object reading
+ *        from the file.
+ * \param inputp The functions puts the pointer to the new input object
+ *               at the address specified by \p inputp.
+ * \param file Name of the configuration file.
+ * \param include_paths Optional, addtional directories to search the file.
+ * \return Zero if successful, otherwise a negative error code.
+ *
+ * This function will search and open the file in the following order
+ * of priority:
+ * 1. directly open the file by its name;
+ * 2. search for the file name in top configuration directory
Quoted 8 lines

src/conf.c static inline void snd_config_unlock(void) { } · line 579

Quoted 6 lines
src/conf.c +6 -0
+ *     "usr/share/alsa/";
+ * 3. search for the file name in in additional configuration directories
+ *     specified by users, via alsaconf syntax
+ *     <confdir:/relative-path/to/user/share/alsa>;
+ *     These directories should be subdirectories of /usr/share/alsa.
+ */

Ditto.

Okay.

Quoted 10 lines
Quoted 10 lines
src/conf.c +10 -0
+static int input_stdio_open(snd_input_t **inputp, const char *file,
+			    struct list_head *include_paths)
+{
+	struct list_head *pos, *npos, *base;
+	struct include_path *path;
+	char *full_path = NULL;
+	int err = 0;
+
+	err = snd_input_stdio_open(inputp, file, "r");
+	if (err == 0)
Quoted 8 lines

src/conf.c static inline void snd_config_unlock(void) { } · line 594

Quoted 5 lines
src/conf.c +5 -0
+		goto out;
+
+	full_path = calloc(1, PATH_MAX + 1);
+	if (!full_path)
+		return -ENOMEM;

In user-space we have a bigger stack, so you can just use the auto variable here instead of heap. It's more efficient.

Okay, I'll do this. Thanks for the tips :-)

Quoted 11 lines

src/conf.c static inline void snd_config_unlock(void) { } · line 599

Quoted 6 lines
src/conf.c +5 -0
+
+	/* search file in top configuration directory usr/share/alsa */
+	strcpy(full_path, ALSA_CONFIG_DIR);
+	strcat(full_path, "/");
+	strcat(full_path, file);

This may overflow. Do like

	snprintf(full_path, sizeof(full_path), "%s/%s",
		ALSA_CONFIG_DIR, file);

Yes, there is the risk and snprintf looks nice. I'll fix as you suggested.

Quoted 4 lines
Quoted 4 lines
src/conf.c +4 -0
+	err = snd_input_stdio_open(inputp, full_path, "r");
+	if (err == 0)
+		goto out;
+
Quoted 8 lines

src/conf.c static inline void snd_config_unlock(void) { } · line 609

Quoted 6 lines
src/conf.c +6 -0
+	/* search file in user specified include paths. These directories
+	 * are subdirectories of /usr/share/alsa.
+	 */
+	if (include_paths) {
+		base = include_paths;
+		list_for_each_safe(pos, npos, base) {

No need to use *_safe version here, as we don't remove the list entry.

Okay. And I'll also use snprintf here.

Quoted 45 lines
Quoted 34 lines
src/conf.c +21 -1
+			path = list_entry(pos, struct include_path, list);
+			if (!path->dir)
+				continue;
+
+			strncpy(full_path, path->dir, PATH_MAX);
+			strcat(full_path, "/");
+			strcat(full_path, file);
+			err = snd_input_stdio_open(inputp, full_path, "r");
+			if (err == 0)
+			goto out;
+		}
+	}
+
+out:
+	if (full_path)
+		free(full_path);
+
+	return err;
+}
+
  static int safe_strtoll(const char *str, long long *val)
  {
  	long long v;
@@ -632,7 +758,7 @@ static int get_char_skip_comments(input_t *input)
  			int err = get_delimstring(&str, '>', input);
  			if (err < 0)
  				return err;
-			if (!strncmp(str, "confdir:", 8)) {
+			if (!strncmp(str, "confdir:", 8)) { /* directory */
  				char *tmp = malloc(strlen(ALSA_CONFIG_DIR) + 1 + strlen(str + 8) + 1);
  				if (tmp == NULL) {
  					free(str);

src/conf.c static int get_char_skip_comments(input_t *input) · line 773

Quoted 7 lines
src/conf.c +4 -0
@@ -641,8 +767,14 @@ static int get_char_skip_comments(input_t *input)
  				sprintf(tmp, ALSA_CONFIG_DIR "/%s", str + 8);
  				free(str);
  				str = tmp;
+
+				err = snd_input_stdio_open(&in, str, "r");
+				if (err == 0)
+					add_include_path(input->current, str);

See my comment above.

Quoted 3 lines

thanks,

Takashi

Thank you very much for the review and comments! I've fixed the issues in the series v4. Please review.

Mengdong

Quoted 43 lines
Quoted 42 lines
src/conf.c +8 -2
+			} else { /* file, also search in include paths */
+				err = input_stdio_open(&in, str,
+						&input->current->include_paths);
  			}
-			err = snd_input_stdio_open(&in, str, "r");
  			if (err < 0) {
  				SNDERR("Cannot access file %s", str);
  				free(str);
@@ -658,6 +790,7 @@ static int get_char_skip_comments(input_t *input)
  			fd->next = input->current;
  			fd->line = 1;
  			fd->column = 0;
+			INIT_LIST_HEAD(&fd->include_paths);
  			input->current = fd;
  			continue;
  		}
@@ -1668,6 +1801,7 @@ static int snd_config_load1(snd_config_t *config, snd_input_t *in, int override)
  	fd->line = 1;
  	fd->column = 0;
  	fd->next = NULL;
+	INIT_LIST_HEAD(&fd->include_paths);
  	input.current = fd;
  	input.unget = 0;
  	err = parse_defs(config, &input, 0, override);
@@ -1708,9 +1842,12 @@ static int snd_config_load1(snd_config_t *config, snd_input_t *in, int override)
  		fd_next = fd->next;
  		snd_input_close(fd->in);
  		free(fd->name);
+		free_include_paths(fd);
  		free(fd);
  		fd = fd_next;
  	}
+
+	free_include_paths(fd);
  	free(fd);
  	return err;
  }
--

2.5.0

#5 lore raw

PATCH v2 3/3 topology: Add doc for including other files in the text conf file

In reply to [email protected]

From: Mengdong Lin <[email protected]>

This patch adds document about how to include other files in the text configuration files, by alsaconf syntax <xxx> and <confdir:xxx>.

Users may define common info in separate files (e.g. vendor tokens, tuples) and share them for different platforms, by including them via syntax <path/to/configuration-file>. This can save the total size of files. Users can also specifiy additional configuraiton directories relative to "usr/share/alsa/" to search the included files, via syntax <confdir:/relative-path/to/usr/share/alsa>.

The alsaconf will search and open an included file in the following order of priority:
1. directly open the file by its name;
2. search for the file name in "usr/share/alsa";
3. search for the file name in user specified subdirectories under "usr/share/alsa".

The order of the included files need not to be same as their dependencies, because the toplogy library will load all of them before parsing their dependencies.

include/topology.h +30 -0
index a0d018e..c3449fd 100644
@@ -624,6 +624,36 @@ extern "C" {
  *	data "name"			# optional private data
  * }
  * </pre>
+ *
+ * <h4>Include other files</h4>
+ * Users may include other files in a text conf file via alsaconf syntax
+ * <path/to/configuration-file>. This allows users to define common info
+ * in separate files (e.g. vendor tokens, tuples) and share them for
+ * different platforms, thus save the total size of config files. <br>
+ * Users can also specifiy additional configuraiton directories relative
+ * to "usr/share/alsa/" to search the included files,  via alsaconf syntax
+ * <confdir:/relative-path/to/usr/share/alsa>. <br><br>
+ *
+ * For example, file A and file B are two text conf files for platform X,
+ * they will be installed to /usr/share/alsa/topology/platformx. If we
+ * need file A to include file B, in file A we can add: <br>
+ *
+ * <confdir:topology/platformx> <br>
+ * <name-of-file-B> <br><br>
+ *
+ * ALSA conf will search and open an included file in the following order
+ * of priority:
+ *  1. directly open the file by its name;
+ *  2. search for the file name in "usr/share/alsa";
+ *  3. search for the file name in user specified subdirectories under
+ *     "usr/share/alsa".
+ *
+ * The order of the included files need not to be same as their
+ * dependencies, since the topology library will load them all before
+ * parsing their dependencies. <br>
+ *
+ * The configuration directories defined by a file will only be used to search
+ * the files included by this file.
  */
 
 /** Maximum number of channels supported in one control */
Signature · 1 line
2.5.0