[PATCH 3/4] config-parser: Enable updating an already-loaded config

Bryce Harrington bryce at osg.samsung.com
Wed Jan 21 17:07:51 PST 2015


On Wed, Jan 21, 2015 at 01:08:21PM -0600, Derek Foreman wrote:
> On 20/01/15 05:30 PM, Bryce Harrington wrote:
> > From: "Bryce Harrington" <bryce at osg.samsung.com>
> > 
> > Adds weston_config_update(), which parses a condensed string of
> > comma-delimited key/value pairs and adds or changes the corresponding
> > parameters in the weston_config structure.
> > 
> > Signed-off-by: Bryce Harrington (http://osg.samsung.com) <bryce at osg.samsung.com>
> > Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> > ---
> >  shared/config-parser.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++---
> >  shared/config-parser.h |  6 +++++
> >  2 files changed, 75 insertions(+), 3 deletions(-)
> > 
> > diff --git a/shared/config-parser.c b/shared/config-parser.c
> > index 78039ba..0b60c2f 100644
> > --- a/shared/config-parser.c
> > +++ b/shared/config-parser.c
> > @@ -377,6 +377,20 @@ weston_config_get_libexec_dir(void)
> >  }
> >  
> >  struct weston_config *
> > +weston_config_create(void)
> > +{
> > +	struct weston_config *config;
> > +
> > +	config = malloc(sizeof *config);
> > +	if (config == NULL)
> > +		return NULL;
> > +
> > +	wl_list_init(&config->section_list);
> > +
> > +	return config;
> > +}
> 
> The need for this isn't explained in the commit log... is this intended
> to be part of a different patch?

This is just splitting the creation logic out from
weston_config_parse(), so it can be used in compositor.c in the next
patch.
 
> > +/* Parses a string of the form section.key=value,section.key=value
> > + * and updates the provided config.
> > + */
> > +bool
> > +weston_config_update(struct weston_config *config, char *line)

Thanks for reviewing the patchset.  I've made the changes in my local
tree.

But after chatting with you on IRC about patch 4, I'm increasingly
thinking this solution might not be the best way to tackle this.  As you
pointed out, the syntax is going to be inconsistent with the weston.ini,
and there's some built-in limitations like not being able to create
multiple monitor sections.  Since one part of the motivation for the
headless testing is to enable synthetic testing of multi-monitor
functionality, that's a pretty severe limitation.

But beyond this I'm realizing this approach creates some uncertainty.
We start with some arbitrary weston.ini, then apply some changes to it
via the command line; we don't have a way to easily review what the
final configuration was (we could dump the config to log, but that's yet
more code...)

The alternate solution was to just generate weston.ini files from the
tests themselves, and then pass them via a --config-file or
--config-path parameter.  This sidesteps all the formatting issues and
limitations.  More importantly, we can directly view the configuration
we're sending in.

We didn't pursue that approach originally because it sounded like it
could result in a lot of extra annoying test maintenance work.  But if
we can keep the test weston.ini's minimal (i.e. just a section name and
the single parameter to change) it shouldn't be that bad.  If our needs
become more than minimal, we can consider adding some sort of weston.ini
file builder.

Bryce


More information about the wayland-devel mailing list