On Sun, May 12, 2013 at 11:54 AM, Kristian Høgsberg <k...@bitplanet.net> wrote:
> 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);
Oops, hit send too early, the snprintf is supposed to be:
snprintf(path, size, "%.*s/weston/%s", next - p, p, name);
and after open I'm of course missing:
if (fd >=0)
return fd;
}
return -1;
}
and with that we can change the parse function to take an fd and fseek
it to reset the pointer to the start of the file. Once we integrate
the weston_config stuff, we can make open_config_file static and stop
exporting it, since then all config file access will have to go to the
compositor->config object.
Kristian
>> +
>> + 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
