On Thu, 20 Oct 2016 07:48:37 +0200,
mengdong.lin@xxxxxxxxxxxxxxx wrote:
>
> From: Mengdong Lin <mengdong.lin@xxxxxxxxxxxxxxx>
>
> 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>;
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.
> The configuration directories defined by a file will only be used to search
> the files included by this file.
>
> Signed-off-by: Mengdong Lin <mengdong.lin@xxxxxxxxxxxxxxx>
>
> diff --git a/src/conf.c b/src/conf.c
> index a516611..44ef673 100644
> --- a/src/conf.c
> +++ b/src/conf.c
> @@ -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/".
Don't use the doxygen comment for static functions.
They shouldn't be included in the exported documents.
> + */
> +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.
> + 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) {
...
> + 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.
> +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.
> + */
Ditto.
> +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;
In user-space we have a bigger stack, so you can just use the auto
variable here instead of heap. It's more efficient.
> +
> + /* 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);
> + 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.
> + 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);
See my comment above.
thanks,
Takashi
> + } 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
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
