[pulseaudio-discuss] [PATCH] conf-parser: add support for .d directories

[Peter Meerwald]

> > > > > +                filename2 = pa_sprintf_malloc("%s" PA_PATH_SEP "%s", dir_name, entries[i]->d_name);
> > > > > +                pa_config_parse(filename2, NULL, t, proplist, false, userdata);
> > > > 
> > > > the return value is not checked;
> > > > for config files included with .include, the retval is checked
> > > 
> > > In my opinion pa_config_parse() shouldn't even return anything. A single
> > > fault in the client.conf shouldn't prevent all clients from connecting
> > > to the server, and reverting to the default configuration isn't a good
> > > option either. A single configuration fault should only result in a
> > > single error message in the log, and the configuration parsing should
> > > continue as usual after the error.
> > > 
> > > So, I think when config files are included with .include, the return
> > > value of pa_config_parse() should not be checked.
> > 
> > $ rgrep pa_config_parse\( .
> > ./pulsecore/conf-parser.c:        r = pa_config_parse(fn, NULL, state->item_table, state->proplist, state->userdata);
> > ./pulsecore/conf-parser.c:int pa_config_parse(const char *filename, FILE *f, const pa_config_item *t, pa_proplist *proplist, void *userdata) {
> > ./modules/alsa/alsa-mixer.c:    r = pa_config_parse(fn, NULL, items, p->proplist, p);
> > ./modules/alsa/alsa-mixer.c:    r = pa_config_parse(fn, NULL, items, NULL, ps);
> > ./modules/module-augment-properties.c:    if (pa_config_parse(fn, NULL, table, NULL, r) < 0)
> > ./daemon/daemon-conf.c:    r = f ? pa_config_parse(c->config_file, f, table, NULL, NULL) : 0;
> > ./pulse/client-conf.c:        pa_config_parse(fn, f, table, NULL, NULL);
> > 
> > client-conf does not check the retval, everywhere else it is checked
> > (not saying that one way is better than the other, but probably the 
> > decision to ignore the retval should be with the caller, not internally 
> > in pa_config_parse())
> 
> Well, in my opinion each of the callers should choose to ignore the
> return value, and if that happens, then the return value will be
> redundant.

sound good, let's have consistent behaviour :)

p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)