[PATCH] Add new config parser
Kristian Høgsberg
hoegsberg at gmail.com
Mon Apr 1 11:10:56 PDT 2013
On Mon, Apr 01, 2013 at 07:29:08PM +0200, Giulio Camuffo wrote:
> Sorry, i was a bit too rushing. I made a couple of comments below.
>
>
> 2013/4/1 Kristian Høgsberg <krh at bitplanet.net>
>
> > The current config parser, parses the ini file and pulls out the values
> > specified by the struct config_section passed to parse_config_file() and
> > then throw the rest away. This means that every place we want to get
> > info out of the ini file, we have to parse the whole thing again. It's not
> > a big overhead, but it's also not a convenient API.
> >
> > This patch adds a parser that parses the ini file to a data structure and
> > puts that in weston_compositor->config along with API to query comfig
> > keys from the data structure. The old parser is still available, but
> > we'll transition to the new approach over the next few commits.
> > ---
> > shared/config-parser.c | 225
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> > shared/config-parser.h | 26 ++++++
> > src/compositor.c | 2 +
> > src/compositor.h | 1 +
> > 4 files changed, 254 insertions(+)
> >
> > diff --git a/shared/config-parser.c b/shared/config-parser.c
> > index 10ff86a..5582cc4 100644
> > --- a/shared/config-parser.c
> > +++ b/shared/config-parser.c
> > @@ -25,7 +25,9 @@
> > #include <stdlib.h>
> > #include <assert.h>
> > #include <ctype.h>
> > +#include <errno.h>
> >
> > +#include <wayland-util.h>
> > #include "config-parser.h"
> >
> > static int
> > @@ -185,3 +187,226 @@ config_file_path(const char *name)
> > snprintf(path, size, "%s/%s", config_dir, name);
> > return path;
> > }
> > +
> > +struct weston_config_entry {
> > + char *key;
> > + char *value;
> > + struct wl_list link;
> > +};
> > +
> > +struct weston_config_section {
> > + char *name;
> > + struct wl_list entry_list;
> > + struct wl_list link;
> > +};
> > +
> > +struct weston_config {
> > + struct wl_list section_list;
> > +};
> > +
> > +struct weston_config_section *
> > +weston_config_get_section(struct weston_config *config, const char
> > *section)
> > +{
> > + struct weston_config_section *s;
> > +
> > + wl_list_for_each(s, &config->section_list, link)
> > + if (strcmp(s->name, section) == 0)
> > + return s;
> > +
> > + return NULL;
> > +}
> > +
> > +static struct weston_config_entry *
> > +config_section_get_entry(struct weston_config_section *section,
> > + const char *key)
> > +{
> > + struct weston_config_entry *e;
> > +
> > + if (section == NULL)
> > + return NULL;
> > + wl_list_for_each(e, §ion->entry_list, link)
> > + if (strcmp(e->key, key) == 0)
> > + return e;
> > +
> > + return NULL;
> > +}
> > +
> > +int
> > +weston_config_section_get_int(struct weston_config_section *section,
> > + const char *key,
> > + int32_t *value, int32_t default_value)
> > +{
> > + struct weston_config_entry *entry;
> > + char *end;
> > +
> > + entry = config_section_get_entry(section, key);
> > + if (entry == NULL) {
> > + *value = default_value;
> > + errno = ENOENT;
> > + return -1;
> > + }
> > +
> > + *value = strtol(entry->value, &end, 0);
> > + if (*end != '\0') {
> > + *value = default_value;
> > + errno = EINVAL;
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int
> > +weston_config_section_get_uint(struct weston_config_section *section,
> > + const char *key,
> > + uint32_t *value, uint32_t default_value)
> > +{
> > + struct weston_config_entry *entry;
> > + char *end;
> > +
> > + entry = config_section_get_entry(section, key);
> > + if (entry == NULL) {
> > + *value = default_value;
> > + errno = ENOENT;
> > + return -1;
> > + }
> > +
> > + *value = strtoul(entry->value, &end, 0);
> > + if (*end != '\0') {
> > + *value = default_value;
> > + errno = EINVAL;
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int
> > +weston_config_section_get_string(struct weston_config_section *section,
> > + const char *key,
> > + const char **value, const char
> > *default_value)
> > +{
> > + struct weston_config_entry *entry;
> > +
> > + entry = config_section_get_entry(section, key);
> > + if (entry == NULL) {
> > + if (default_value)
> > + *value = strdup(default_value);
> > + errno = ENOENT;
> > + return -1;
> > + }
> > +
> > + *value = strdup(entry->value);
> > +
> > + return 0;
> > +}
> > +
> > +int
> > +weston_config_section_get_bool(struct weston_config_section *section,
> > + const char *key,
> > + int *value, int default_value)
> > +{
> > + struct weston_config_entry *entry;
> > +
> > + entry = config_section_get_entry(section, key);
> > + if (entry == NULL) {
> > + *value = default_value;
> > + errno = ENOENT;
> > + return -1;
> > + }
> > +
> > + if (strcmp(entry->value, "false\n") == 0)
> > + *value = 0;
> > + else if (strcmp(entry->value, "true\n") == 0)
> > + *value = 1;
> > + else {
> > + *value = default_value;
> > + errno = EINVAL;
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static struct weston_config_section *
> > +config_add_section(struct weston_config *config, const char *name)
> > +{
> > + struct weston_config_section *section;
> > +
> > + section = malloc(sizeof *section);
> > + section->name = strdup(name);
> > + wl_list_init(§ion->entry_list);
> > + wl_list_insert(config->section_list.prev, §ion->link);
> > +
> > + return section;
> > +}
> > +
> > +static struct weston_config_entry *
> > +section_add_entry(struct weston_config_section *section,
> > + const char *key, const char *value)
> > +{
> > + struct weston_config_entry *entry;
> > +
> > + entry = malloc(sizeof *entry);
> > + entry->key = strdup(key);
> > + entry->value = strdup(value);
> > + wl_list_insert(section->entry_list.prev, &entry->link);
> > +
> > + return entry;
> > +}
> > +
> > +struct weston_config *
> > +weston_config_parse(const char *path)
> > +{
> > + FILE *fp;
> > + char line[512], *p;
> > + struct weston_config *config;
> > + struct weston_config_section *section = NULL;
> > + int i;
> > +
> > + config = malloc(sizeof *config);
> > + if (config == NULL) {
> > + free(path);
> >
>
> This shouldn't free the path. It is a const char* and could well be a
> literal. The user should free it itself if it is the case.
Right, thanks. I had the call to config_file_path() as part of
weston_config_parse() initially for convenience, but we already look
that up in weston_compositor_init() and it makes it hard to write
tests for the parser. I missed this error path when I pulled it out.
> + return NULL;
> > + }
> > +
> > + wl_list_init(&config->section_list);
> > +
> > + fp = fopen(path, "r");
> > + if (fp == NULL) {
> > + fprintf(stderr, "couldn't open %s\n", path);
> > + free(config);
> > + return NULL;
> > + }
> > +
> > + while (fgets(line, sizeof line, fp)) {
> > + if (line[0] == '#' || line[0] == '\n') {
> > + continue;
> > + } if (line[0] == '[') {
> > + p = strchr(&line[1], ']');
> > + if (!p || p[1] != '\n') {
> > + fprintf(stderr, "malformed "
> > + "section header: %s\n", line);
> > + fclose(fp);
> > + free(config);
> > + return NULL;
> > + }
> > + p[0] = '\0';
> > + section = config_add_section(config, &line[1]);
> > + } else if (p = strchr(line, '='), p && section) {
> > + p[0] = '\0';
> > + i = strlen(&p[1]);
> > + p[i + 1] = '\0';
> > + section_add_entry(section, line, &p[1]);
> >
>
> This is also putting the newline in the value of the entry.
Yeah, lost that when I copied over handle_valued()... Ok, I'll repost
this again once I have some tests to go with it.
Kristian
> + } else {
> > + fprintf(stderr, "malformed config line: %s\n",
> > line);
> > + fclose(fp);
> > + free(config);
> > + return NULL;
> > + }
> > + }
> > +
> > + fclose(fp);
> > +
> > + return config;
> > +}
> > diff --git a/shared/config-parser.h b/shared/config-parser.h
> > index 1d0ee3f..0efbac3 100644
> > --- a/shared/config-parser.h
> > +++ b/shared/config-parser.h
> > @@ -73,6 +73,32 @@ int
> > parse_options(const struct weston_option *options,
> > int count, int *argc, char *argv[]);
> >
> > +struct weston_config_section;
> > +struct weston_config;
> > +
> > +struct weston_config_section *
> > +weston_config_get_section(struct weston_config *config, const char
> > *section);
> > +int
> > +weston_config_section_get_int(struct weston_config_section *section,
> > + const char *key,
> > + int32_t *value, int32_t default_value);
> > +int
> > +weston_config_section_get_uint(struct weston_config_section *section,
> > + const char *key,
> > + uint32_t *value, uint32_t default_value);
> > +int
> > +weston_config_section_get_string(struct weston_config_section *section,
> > + const char *key,
> > + const char **value,
> > + const char *default_value);
> > +int
> > +weston_config_section_get_bool(struct weston_config_section *section,
> > + const char *key,
> > + int *value, int default_value);
> > +struct weston_config *
> > +weston_config_parse(const char *basename);
> > +
> > +
> > #ifdef __cplusplus
> > }
> > #endif
> > diff --git a/src/compositor.c b/src/compositor.c
> > index 1617d96..cb374f7 100644
> > --- a/src/compositor.c
> > +++ b/src/compositor.c
> > @@ -3139,6 +3139,8 @@ weston_compositor_init(struct weston_compositor *ec,
> > keyboard_config_keys,
> > ARRAY_LENGTH(keyboard_config_keys) },
> > };
> >
> > + ec->config = weston_config_parse(config_file);
> > +
> > memset(&xkb_names, 0, sizeof(xkb_names));
> > parse_config_file(config_file, cs, ARRAY_LENGTH(cs), ec);
> >
> > diff --git a/src/compositor.h b/src/compositor.h
> > index 7d1d68e..e4c7ab2 100644
> > --- a/src/compositor.h
> > +++ b/src/compositor.h
> > @@ -301,6 +301,7 @@ struct weston_compositor {
> >
> > struct wl_display *wl_display;
> > struct weston_shell_interface shell_interface;
> > + struct weston_config *config;
> >
> > struct wl_signal activate_signal;
> > struct wl_signal kill_signal;
> > --
> > 1.8.1.2
> >
> >
More information about the wayland-devel
mailing list