[PATCH] config-parser: Honor the XDG_CONFIG_DIRS environment variable

Kristian Høgsberg hoegsberg at gmail.com
Tue May 14 07:05:41 PDT 2013


On Mon, May 13, 2013 at 11:41 PM, Othman, Ossama
<ossama.othman at intel.com> wrote:
> Hi Kristian,
>
>> I think it's good to go - however, inlining the patch messed up the
>> whitespace.  Ideally, send patches using git send-email, which can be
>> configured to use smtp servers, and you can pass --annotate if you
>> want to add a message or comment to the patch (put it after the ---
>> that indicates the end of the commit message).  Or as a last resort,
>> attach the patch.
>
>
> I forgot to attach the patch this last time around.  Sorry about that.  I'll
> just resend the patch with git send-email, as you suggest.
>
>>
>> As for the next thing, good catch.  I'd do something like
>>
>>     for (p = config_dirs; p != NULL; p = next) {
>>         next = strchrnul(p, ':');
>>         if (*next == ':')
>>             next++;
>>
>>         ...
>>
>> to keep the for (...) more readable.
>
>
> Changing the loop termination to "p != NULL" causes an infinite loop since
> strchnul() returns a pointer to the null character '\0', not the NULL

Yeah, sorry for the confusion, for a second while writing the email I
was thinking that we could just use strchr instead and test for p !=
NULL, but that doesn't work for getting the end of the last path
element.

> pointer.  Also, the moving the next pointer past the colon must be done
> after path construction, other the colon ends up at the end of the path.
> How's this instead:
>
> for (p = config_dirs; *p != '\0'; p = next) {
> next = strchrnul(p, ':');
> snprintf(path, sizeof path,
> "%.*s/weston/%s", (int)(next - p), p, name);
> fd = open(path, O_RDONLY | O_CLOEXEC);
> if (fd >= 0)
> return fd;
>
> if (*next == ':')
> next++;
> }
>
> Note that I also had to cast "next - p" in the snprintf() call to int since
> the format specifier "%*.s" wants an int but the pointer difference on my 64
> bit Ubuntu box is a long int, at least according to the format specifier
> warning I get:
>
> foo.c: In function ‘open_config_file’:
> foo.c:53:5: warning: field precision specifier ‘.*’ expects argument of type
> ‘int’, but argument 4 has type ‘long int’ [-Wformat]
>
> foo.c in this case is a simple test program I wrote.  I assume the cast is
> safe since "next - p" should always fall within the range of type int.
>
> Anyway, I'll resend the patch with the above changes with git send-email.

Yup, that all sounds good.

thanks,
Kristian


More information about the wayland-devel mailing list