[Mesa-dev] [PATCH v2 1/3] xmlconfig: read more config files from drirc.d/

Emil Velikov emil.l.velikov at gmail.com
Fri Aug 3 10:28:36 UTC 2018


Hi Qiang Yu,

Couple of minor suggestions.

On 3 August 2018 at 09:19, Qiang Yu <Qiang.Yu at amd.com> wrote:
> v2:
>   drop /etc/drirc.d
>
Revision log is at the end of the commit message. See for example
61a02729f749add535ad9d18c62f65641e428cfb - git log has many more.

> Driver and application can put their drirc files in
> ${datadir}/drirc.d/ with name xxx.conf. Config files
> will be read and applied in file name alphabete order.
>
type: alphabetic

> So there are three places for drirc listed in order:
> 1. /usr/share/drirc.d/
> 2. /etc/drirc
> 3. ~/.drirc
>
I would suggest keeping the refactor, separate, from the newly
introduced path + directory handling.


> +/** \brief Parse configuration files in a directory */
> +static void
> +parseConfigDir(struct OptConfData *data, const char *dirname)
> +{
> +    int i, count;
> +    struct dirent **entries = NULL;
> +
> +    count = scandir(dirname, &entries, scandir_filter, alphasort);
> +    if (count < 0)
We have nothing to do with 0 entries, so make it < 1?
Or just keep it as-is, if entries still needs to be free'd when 0.

> +        return;
> +
> +    for (i = 0; i < count; i++) {
> +        char filename[PATH_MAX];
> +
> +        snprintf(filename, PATH_MAX, "%s/%s", dirname, entries[i]->d_name);
free(entries[i]);

> +        parseOneConfigFile(data, filename);
> +    }
> +
> +    if (entries)
It should always be true?

> +        free(entries);
> +}

HTH
Emil


More information about the mesa-dev mailing list