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

[Julien Cristau]

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;
+    }
+    if (!start) {
+        temp_path = Xprintf("%s%sbuilt-ins", path, *path ? "," : "");
     } else {
         temp_path = Xstrdup(path);
     }

Cheers,
Julien