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

[Jon TURNEY]

On 08/09/2009 10:25, Rémi Cardona wrote:
> 49b93df8a3002db7196aa3fc1fd8dca1c12a55d6 made the hard dependency on
> a "fixed" font go away but only Xorg could use the built-ins fonts by
> default.
>
> With this commit, all DDXs get "built-ins" appended to their FontPath, not
> just Xorg.
>
> Tested with Xorg, Xvfb and Xnest.

Thanks for this patch, I have been meaning to clean up my similar patch and 
push it.  See below for what I got stuck on...

>
> Signed-off-by: Rémi Cardona<remi at gentoo.org>
> ---
>   dix/dixfonts.c                 |   20 ++++++++++++++++++--
>   hw/xfree86/common/xf86Config.c |   16 ----------------
>   2 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/dix/dixfonts.c b/dix/dixfonts.c
> index 719bca4..4d69025 100644
> --- a/dix/dixfonts.c
> +++ b/dix/dixfonts.c
> @@ -1818,6 +1818,9 @@ SetDefaultFontPath(char *path)
>       unsigned char *cp,
>                  *pp,
>                  *nump,
> +               *temp_path,
> +               *start,
> +               *end,
>                  *newpath;
>       int         num = 1,
>                   len,
> @@ -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

I'm not sure if FPEs don't get uniquified later on anyhow, or if it is benign 
to duplicate FPEs, if so this complexity isn't needed and we could just add 
"built-ins" at the end and be done with it...

>       /* get enough for string, plus values -- use up commas */
> -    len = strlen(path) + 1;
> +    len = strlen(temp_path) + 1;
>       nump = cp = newpath = (unsigned char *) xalloc(len);
>       if (!newpath)
>   	return BadAlloc;
> -    pp = (unsigned char *) path;
> +    pp = (unsigned char *) temp_path;
>       cp++;
>       while (*pp) {
>   	if (*pp == ',') {
> @@ -1849,6 +1864,7 @@ SetDefaultFontPath(char *path)
>       err = SetFontPathElements(num, newpath,&bad, TRUE);
>
>       xfree(newpath);
> +    xfree(temp_path);
>
>       return err;
>   }
> diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c
> index 9376119..ddf4745 100644
> --- a/hw/xfree86/common/xf86Config.c
> +++ b/hw/xfree86/common/xf86Config.c
> @@ -612,22 +612,6 @@ configFiles(XF86ConfFilesPtr fileconf)
>   	pathFrom = X_DEFAULT;
>       temp_path = defaultFontPath ? defaultFontPath : "";
>
> -    /* ensure defaultFontPath contains "built-ins" */
> -    start = strstr(temp_path, "built-ins");
> -    end = start + strlen("built-ins");
> -    if (start == NULL ||
> -	!((start == temp_path || start[-1] == ',')&&  (!*end || *end == ','))) {
> -	defaultFontPath = Xprintf("%s%sbuilt-ins",
> -				  temp_path, *temp_path ? "," : "");
> -	if (must_copy == TRUE) {
> -	    if (defaultFontPath != NULL) {
> -		must_copy = FALSE;
> -	    }
> -	} else {
> -	    /* already made a copy of the font path */
> -	    xfree(temp_path);
> -	}
> -    }
>       /* xf86ValidateFontPath modifies its argument, but returns a copy of it. */
>       temp_path = must_copy ? XNFstrdup(defaultFontPath) : defaultFontPath;
>       defaultFontPath = xf86ValidateFontPath(temp_path);