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 <[email protected]>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 <[email protected]>
> > 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
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
