[PATCH] config-parser: Honor the XDG_CONFIG_DIRS environment variable

Othman, Ossama ossama.othman at intel.com
Sun May 12 19:33:44 PDT 2013


Hi Kristian,

On Sun, May 12, 2013 at 8:54 AM, Kristian Høgsberg <krh at bitplanet.net>wrote:
>
> I think we can change the interface to int open_config_file(const char
> *), which looks up and opens the config file and returns the fd.  We
> can keep that in weston_compositor instead of the config_file path.
> The parse function can then fseek on the fd to reset to the beginning
> of the file.  Going forward, we'll do something like this:
>
>
> http://lists.freedesktop.org/archives/wayland-devel/2013-April/008333.html


Nice set of changes!


> and store the weston_config object in weston_compositor object.  The
> config path patch here is somewhat orthogonal to that though.
>
> On Sun, May 12, 2013 at 2:20 AM, Othman, Ossama <ossama.othman at intel.com>
> wrote:
>
...

> > + info.count = 0;
> > + info.paths = malloc(DEFAULT_DIR_COUNT * sizeof(char *));
> > + if (info.paths == NULL)
> > + return NULL;
>
> I don't think this function should need any mallocs at all.


Agreed.  I was a bit too loose with the malloc()s.  :)

Basically
> we should be able to say something like
>
>   char path[PATHMAX];
>
>   snprintf(path, size, "%s%s%s", home_dir, dotconf, name);
>   fd = open(path);
>   if (fd >= 0)
>     return fd;
>
>   /* same for XDG_CONFIG_DIR */
>
> as for XDG_CONFIG_PATHS, I'd just scan through the string manually
> with strchrnul instead of bothering with copying and strtok_r:
>
>   for (p = config_dirs, *p; p = next) {
>     next = strchrnul(p, ':');
>     snprintf(path, size, "%s/weston/%s", p next - p, name);
>     fd = open(path);


That is indeed much better.  Since your new parser is pending would you
like me to hold off submitting a reworked patch that incorporates your
suggestions?

Thanks to both you and Bill for the excellent feedback!
-Ossama
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130512/57483312/attachment.html>


More information about the wayland-devel mailing list