[PATCH] Improve the check for "built-ins" in fontpath

Alan Coopersmith Alan.Coopersmith at Sun.COM
Mon Sep 14 10:44:51 PDT 2009


Why split the string twice in this function?  Why not something
more like (untested, diff vs. master):

--- a/dix/dixfonts.c
+++ b/dix/dixfonts.c
@@ -1818,10 +1818,14 @@ SetDefaultFontPath(char *path)
                 len,
                 err,
                 size = 0,
+               builtins_seen = 0,
                 bad;

+#define BUILTIN_PATH    "built-ins"
+#define BUILTIN_PATH_LEN 9
+
     /* get enough for string, plus values -- use up commas */
-    len = strlen(path) + 1;
+    len = strlen(path) + BUILTIN_PATH_LEN + 1;
     nump = cp = newpath = xalloc(len);
     if (!newpath)
        return BadAlloc;
@@ -1829,6 +1833,8 @@ SetDefaultFontPath(char *path)
     cp++;
     while (*pp) {
        if (*pp == ',') {
+           if (strncmp(nump+1, BUILTIN_PATH,  BUILTIN_PATH_LEN) == 0)
+               builtins_seen = 1;
            *nump = (unsigned char) size;
            nump = cp++;
            pp++;
@@ -1840,6 +1846,11 @@ SetDefaultFontPath(char *path)
        }
     }
     *nump = (unsigned char) size;
+    if (!builtins_seen) {
+       *cp++ = BUILTIN_PATH_LEN;
+       memcpy(cp, BUILTIN_PATH, BUILTIN_PATH_LEN);
+       num++;
+    }

     err = SetFontPathElements(num, newpath, &bad, TRUE);


There's also the question of whether xset fp should be able to
remove built-ins from the path - perhaps the whole thing should
move into SetFontPathElements, so it's handled every time the font
path is set.

	-alan-

Jon TURNEY wrote:
> Improve check for an existing "built-ins" in the fontpath so it
> doesn't get misled if "built-ins" exists as a substring of a path
> or hostname.
> 
> Also fix some signedness mismatch warnings in that code
> 
> Signed-off-by: Jon TURNEY <jon.turney at dronecode.org.uk>
> ---
>  dix/dixfonts.c |   42 ++++++++++++++++++++++++++++--------------
>  1 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/dix/dixfonts.c b/dix/dixfonts.c
> index c8b7bdc..398782b 100644
> --- a/dix/dixfonts.c
> +++ b/dix/dixfonts.c
> @@ -1813,27 +1813,41 @@ SetDefaultFontPath(char *path)
>      unsigned char *cp,
>                 *pp,
>                 *nump,
> -               *temp_path,
> -               *start,
> -               *end,
>                 *newpath;
> +    char *start,
> +               *end,
> +               *temp_path;
>      int         num = 1,
>                  len,
>                  err,
>                  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;
> +     /* ensure temp_path contains "built-ins" */
> +     start = path;
> +     while (1) {
> +         end = strstr(start, ",");
> +         if (end == NULL)
> +           end = start + strlen(start);
> +
> +         if ((end - start) == strlen("built-ins") && strncmp(start, "built-ins", strlen("built-ins")) == 0)
> +           break;
> +
> +         if (*end == '\0')
> +           {
> +             start = NULL;
> +             break;
> +           }
> +
> +         start = end + 1;
> +     }
> +     if (!start) {
> +         temp_path = Xprintf("%s%sbuilt-ins", path, *path ? "," : "");
> +     } else {
> +         temp_path = Xstrdup(path);
> +     }
> +     if (!temp_path)
> +         return BadAlloc;
>  
>      /* get enough for string, plus values -- use up commas */
>      len = strlen(temp_path) + 1;

-- 
	-Alan Coopersmith-           alan.coopersmith at sun.com
	 Sun Microsystems, Inc. - X Window System Engineering



More information about the xorg-devel mailing list