Hi Ossama, 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 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.oth...@intel.com> wrote: > I've revised the patch to build a list of possible filenames as Bill > suggested. This time around I've included a patch for the man page, as > well. > > Thanks! > -Ossama > > --- > > Search for a given config file in the directories listed in > $XDG_CONFIG_DIRS if it wasn't found in $XDG_CONFIG_HOME or ~/.config. > This allows packages to install custom config files in > /etc/xdg/weston, for example, thus allowing them to avoid dealing with > home directories. > > Signed-off-by: Ossama Othman <ossama.oth...@intel.com> > --- > man/weston.ini.man | 12 +++- > shared/config-parser.c | 155 > +++++++++++++++++++++++++++++++++++++++++------- > 2 files changed, 142 insertions(+), 25 deletions(-) > > diff --git a/man/weston.ini.man b/man/weston.ini.man > index 2287730..d37654a 100644 > --- a/man/weston.ini.man > +++ b/man/weston.ini.man > @@ -24,7 +24,10 @@ server is started: > .nf > .BR "$XDG_CONFIG_HOME/weston.ini " "(if $XDG_CONFIG_HOME is set)" > .BR "$HOME/.config/weston.ini " "(if $HOME is set)" > -.BR "<current dir>/weston.ini " "(if both variables were not set)" > +.B "weston/weston.ini in each" > +.BR "\ \ \ \ $XDG_CONFIG_DIR " "(if $XDG_CONFIG_DIRS is set)" > +.BR "/etc/xdg/weston/weston.ini " "(if $XDG_CONFIG_DIRS is not set)" > +.BR "<current dir>/weston.ini " "(if no variables were set)" > .fi > .RE > .PP > @@ -32,7 +35,12 @@ where environment variable > .B $HOME > is the user's home directory, and > .B $XDG_CONFIG_HOME > -is the user specific configuration directory. > +is the user specific configuration directory, and > +.B $XDG_CONFIG_DIRS > +is a colon > +.B ':' > +delimited listed of configuration base directories, such as > +.BR /etc/xdg-foo:/etc/xdg . > .PP > The > .I weston.ini > diff --git a/shared/config-parser.c b/shared/config-parser.c > index 10ff86a..4ebd453 100644 > --- a/shared/config-parser.c > +++ b/shared/config-parser.c > @@ -25,6 +25,7 @@ > #include <stdlib.h> > #include <assert.h> > #include <ctype.h> > +#include <unistd.h> > > #include "config-parser.h" > > @@ -95,6 +96,9 @@ parse_config_file(const char *path, > const struct config_section *current = NULL; > int i; > > + if (path == NULL) > + return -1; > + > fp = fopen(path, "r"); > if (fp == NULL) { > fprintf(stderr, "couldn't open %s\n", path); > @@ -151,37 +155,142 @@ parse_config_file(const char *path, > return 0; > } > > -char * > -config_file_path(const char *name) > +struct config_info > { > - const char dotconf[] = "/.config/"; > - const char *config_dir; > - const char *home_dir; > + /* Array of config file paths. */ > + char **paths; > + > + /* Number of elements in the "paths" array. */ > + size_t count; > +}; > + > +static char * > +allocate_config_file_path(struct config_info *info, size_t path_size) > +{ > + char **paths; > char *path; > - size_t size; > - > - config_dir = getenv("XDG_CONFIG_HOME"); > - if (!config_dir) { > - home_dir = getenv("HOME"); > - if (!home_dir) { > - fprintf(stderr, "HOME is not set, using cwd.\n"); > - return strdup(name); > + > + if (!info) > + return 0; > + > + path = malloc(path_size); > + if (path) { > + paths = realloc(info->paths, > + (info->count + 1) * sizeof(*(info->paths))); > + if (paths) { > + paths[info->count] = path; > + info->paths = paths; > + ++info->count; > + } else { > + free(path); > + path = NULL; > } > + } > + > + return path; > +} > + > +char * > +config_file_path(const char *name) > +{ > + const char *config_dir = getenv("XDG_CONFIG_HOME"); > + const char *home_dir = getenv("HOME"); > + const char *config_dirs = getenv("XDG_CONFIG_DIRS"); > + static const char weston_dir[] = "/weston/"; > + static const char cwd[] = "./"; > + static const int DEFAULT_DIR_COUNT = 5; > + struct config_info info; > + size_t name_len, size; > + char *path, *dir, *saveptr, *tmp_dirs; > + char **p; > + > + 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. 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); fd = open(path); > + > + name_len = strlen(name); > > - size = strlen(home_dir) + sizeof dotconf + strlen(name); > - path = malloc(size); > - if (!path) > - return NULL; > + /* Precedence is given to config files in the home directory, > + * and then to directories listed in XDG_CONFIG_DIRS and > + * finally to the current working directory. */ > > - snprintf(path, size, "%s%s%s", home_dir, dotconf, name); > - return path; > + /* $XDG_CONFIG_HOME */ > + if (config_dir) { > + size = strlen(config_dir) + 1 + name_len + 1; > + path = allocate_config_file_path(&info, size); > + > + if (path) > + snprintf(path, size, "%s/%s", config_dir, name); > } > > - size = strlen(config_dir) + 1 + strlen(name) + 1; > - path = malloc(size); > - if (!path) > + /* $HOME/.config */ > + if (home_dir) { > + static const char dotconf[] = "/.config/"; > + > + size = strlen(home_dir) + sizeof dotconf + name_len; > + path = allocate_config_file_path(&info, size); > + > + if (path) > + snprintf(path, size, "%s%s%s", > + home_dir, dotconf, name); > + } > + > + /* For each $XDG_CONFIG_DIRS: weston/<config_file> */ > + if (!config_dirs) > + config_dirs = "/etc/xdg"; /* See XDG base dir spec. */ > + > + size = strlen(config_dirs) + 1; > + tmp_dirs = malloc(size); > + if (!tmp_dirs) > return NULL; > > - snprintf(path, size, "%s/%s", config_dir, name); > + /* strtok_r() modifies the first argument. Avoid > + * clobbering the process environment. */ > + strncpy(tmp_dirs, config_dirs, size); > + > + for (dir = strtok_r(tmp_dirs, ":", &saveptr); > + dir != NULL; > + dir = strtok_r(NULL, ":", &saveptr)) { > + size = strlen(dir) + sizeof weston_dir + name_len; > + path = allocate_config_file_path(&info, size); > + > + if (path) > + snprintf(path, size, "%s%s%s", dir, weston_dir, name); > + } > + > + free(tmp_dirs); > + > + /* Current working directory. */ > + size = sizeof cwd + name_len; > + path = allocate_config_file_path(&info, size); > + > + if (path) > + snprintf(path, size, "%s%s", cwd, name); > + > + path = NULL; > + > + /* Search for usable configuration file. */ > + for (p = info.paths; p != info.paths + info.count; ++p) { > + /* Use the first found config file. > + * > + * FIXME: TOCTOU race here. Ideally we should open the > + * file and return a handle to it. */ > + if (!path && access(*p, R_OK) == 0) > + path = *p; > + else > + free(*p); > + } > + > + if (path == info.paths[info.count - 1]) > + fprintf(stderr, "HOME is not set, using cwd.\n"); > + > + free(info.paths); > + > + if (!path) > + fprintf(stderr, > + "config file \"%s\" not found.\n", > + name); > + > return path; > } > -- > 1.7.10.4 > > > _______________________________________________ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel