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

[Kristian Høgsberg]

On Sun, May 12, 2013 at 07:33:44PM -0700, Othman, Ossama wrote:
> 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.  :)

It's not out of concern of performance or memory pressure, it's that
every malloc is a potential error path and something you have to
remember to free.  And in this case, the code becomes simpler so it's
a win-win.

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

'sizeof path' here, not just 'size' btw.

> >     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?

I think we can move forward with your patch immediately.  The reworked
parser will use the same function for looking up the config file, so
we can land that now.

Kristian