[PATCH] dix: append "built-ins" to the font path in SetDefaultFontPath

[Jon TURNEY]

On 08/09/2009 12:51, Julien Cristau wrote:
> On Tue, Sep  8, 2009 at 12:16:12 +0100, Jon TURNEY wrote:
>
>> On 08/09/2009 10:25, Rémi Cardona wrote:
>>> @@ -1825,12 +1828,24 @@ SetDefaultFontPath(char *path)
>>>                    size = 0,
>>>                    bad;
>>>
>>> +    /* ensure temp_path contains "built-ins" */
>>> +    start = strstr(path, "built-ins");
>>> +    end = start + strlen("built-ins");
>>> +    if (start == NULL ||
>>> +	!((start == path || start[-1] == ',')&&   (!*end || *end == ','))) {
>>> +	temp_path = Xprintf("%s%sbuilt-ins", path, *path ? "," : "");
>>> +    } else {
>>> +        temp_path = Xstrdup(path);
>>> +    }
>>> +    if (!temp_path)
>>> +        return BadAlloc;
>>> +
>>
>> I slightly dislike this part.  It could at least do with a better comment,
>> mentioning that it's checking for the element 'built-ins' somewhere in the
>> font-path (not as part of a filename or hostname etc.)
>>
>> But it looks to me like the code will do the wrong thing in (pathological)
>> cases like "/usr/share/fonts/built-ins,built-ins" and append "built-ins" anyhow
>>
> So how about the below on top of Remi's patch?
> (not even build tested, though)
>
> diff --git a/dix/dixfonts.c b/dix/dixfonts.c
> index c8b7bdc..7da95c9 100644
> --- a/dix/dixfonts.c
> +++ b/dix/dixfonts.c
> @@ -1824,11 +1824,17 @@ SetDefaultFontPath(char *path)
>                   bad;
>
>       /* ensure temp_path contains "built-ins" */
> -    start = strstr(path, "built-ins");
> -    end = start + strlen("built-ins");
> -    if (start == NULL ||
> -	!((start == path || start[-1] == ',')&&  (!*end || *end == ','))) {
> -	temp_path = Xprintf("%s%sbuilt-ins", path, *path ? "," : "");
> +    start = path;
> +    while (1) {
> +        start = strstr(start, "built-ins");
> +        if (start == NULL)
> +            break;
> +        end = start + strlen("built-ins");
> +        if (((start == path || start[-1] == ',')&&  (!*end || *end == ',')))
> +            break;
> +    }

This doesn't advance start (and so loops forever) if the font-path contains 
"built-ins" as a non-initial substring of a font-path element.

I'll follow-up with my attempt at this, but since this is easy to get wrong, 
and not really very important, I don't think this is really suitable for 1.7, 
whereas Rémi's original patch probably is (since it would be nice for built-in 
fonts to be correctly added to the fontpath in DDXs other than Xorg)