Bug 17832 - Memory leaks due to FcStrStaticName use for external patterns

Use the reference-counted strings instead of the static strings

Patch from Karl Tomlinson

src/fccfg.c

@@ -1009,7 +1009,7 @@ FcConfigEvaluate (FcPattern *p, FcExpr *e)
 		case FcOpPlus:
 		    v.type = FcTypeString;
 		    str = FcStrPlus (vl.u.s, vr.u.s);
-		    v.u.s = FcStrStaticName (str);
+		    v.u.s = FcSharedStr (str);
 		    FcStrFree (str);
 			
 		    if (!v.u.s)

src/fcinit.c

@@ -139,7 +139,7 @@ FcFini (void)
     if (_fcConfig)
 	FcConfigDestroy (_fcConfig);
 
-    FcPatternFini ();
+    FcObjectFini ();
     FcCacheFini ();
     if (FcDebug() & FC_DBG_MEMORY)
 	FcMemReport ();
@@ -221,7 +221,7 @@ static struct {
     { "vstack" },
     { "attr" },
     { "pstack" },
-    { "staticstr" },
+    { "sharedstr" },
 };
 
 static int  FcAllocCount, FcAllocMem;

src/fcint.h

@@ -103,7 +103,7 @@
 #define FC_MEM_VSTACK	    26
 #define FC_MEM_ATTR	    27
 #define FC_MEM_PSTACK	    28
-#define FC_MEM_STATICSTR    29
+#define FC_MEM_SHAREDSTR    29
 
 #define FC_MEM_NUM	    30
 
@@ -948,14 +948,14 @@ FcPatternObjectGetBool (const FcPattern *p, FcObject object, int n, FcBool *b);
 FcPrivate FcResult
 FcPatternObjectGetLangSet (const FcPattern *p, FcObject object, int n, FcLangSet **ls);
 
-FcPrivate void
-FcPatternFini (void);
-
 FcPrivate FcBool
 FcPatternAppend (FcPattern *p, FcPattern *s);
 
 FcPrivate const FcChar8 *
-FcStrStaticName (const FcChar8 *name);
+FcSharedStr (const FcChar8 *name);
+
+FcPrivate FcBool
+FcSharedStrFree (const FcChar8 *name);
 
 FcPrivate FcChar32
 FcStringHash (const FcChar8 *s);

src/fclist.c

@@ -67,13 +67,16 @@ FcObjectSetAdd (FcObjectSet *os, const char *object)
     low = 0;
     mid = 0;
     c = 1;
-    object = (char *)FcStrStaticName ((FcChar8 *)object);
+    object = (char *)FcSharedStr ((FcChar8 *)object);
     while (low <= high)
     {
 	mid = (low + high) >> 1;
 	c = os->objects[mid] - object;
 	if (c == 0)
+	{
+	    FcSharedStrFree ((FcChar8 *)object);
 	    return FcTrue;
+	}
 	if (c < 0)
 	    low = mid + 1;
 	else
@@ -91,8 +94,13 @@ FcObjectSetAdd (FcObjectSet *os, const char *object)
 void
 FcObjectSetDestroy (FcObjectSet *os)
 {
+    int i;
+
     if (os->objects)
     {
+	for (i = 0; i < os->nobject; i++)
+	    FcSharedStrFree ((FcChar8 *)os->objects[i]);
+
 	FcMemFree (FC_MEM_OBJECTPTR, os->sobject * sizeof (const char *));
 	free ((void *) os->objects);
     }

src/fcname.c

@@ -572,9 +572,10 @@ FcNameBool (const FcChar8 *v, FcBool *result)
 }
 
 static FcValue
-FcNameConvert (FcType type, FcChar8 *string, FcMatrix *m)
+FcNameConvert (FcType type, FcChar8 *string)
 {
     FcValue	v;
+    FcMatrix	m;
 
     v.type = type;
     switch (v.type) {
@@ -583,7 +584,7 @@ FcNameConvert (FcType type, FcChar8 *string, FcMatrix *m)
 	    v.u.i = atoi ((char *) string);
 	break;
     case FcTypeString:
-	v.u.s = FcStrStaticName(string);
+	v.u.s = FcSharedStr (string);
 	if (!v.u.s)
 	    v.type = FcTypeVoid;
 	break;
@@ -595,8 +596,8 @@ FcNameConvert (FcType type, FcChar8 *string, FcMatrix *m)
 	v.u.d = strtod ((char *) string, 0);
 	break;
     case FcTypeMatrix:
-	v.u.m = m;
+	sscanf ((char *) string, "%lg %lg %lg %lg", &m.xx, &m.xy, &m.yx, &m.yy);
-	sscanf ((char *) string, "%lg %lg %lg %lg", &m->xx, &m->xy, &m->yx, &m->yy);
+	v.u.m = FcMatrixCopy (&m);
 	break;
     case FcTypeCharSet:
 	v.u.c = FcNameParseCharSet (string);
@@ -648,7 +649,6 @@ FcNameParse (const FcChar8 *name)
     FcChar8		*e;
     FcChar8		delim;
     FcValue		v;
-    FcMatrix		m;
     const FcObjectType	*t;
     const FcConstant	*c;
 
@@ -699,31 +699,13 @@ FcNameParse (const FcChar8 *name)
 		    name = FcNameFindNext (name, ":,", save, &delim);
 		    if (t)
 		    {
-			v = FcNameConvert (t->type, save, &m);
+			v = FcNameConvert (t->type, save);
 			if (!FcPatternAdd (pat, t->object, v, FcTrue))
 			{
-			    switch (v.type) {
+			    FcValueDestroy (v);
-			    case FcTypeCharSet:
-				FcCharSetDestroy ((FcCharSet *) v.u.c);
-				break;
-			    case FcTypeLangSet:
-				FcLangSetDestroy ((FcLangSet *) v.u.l);
-				break;
-			    default:
-				break;
-			    }
 			    goto bail2;
 			}
-			switch (v.type) {
+			FcValueDestroy (v);
-			case FcTypeCharSet:
-			    FcCharSetDestroy ((FcCharSet *) v.u.c);
-			    break;
-			case FcTypeLangSet:
-			    FcLangSetDestroy ((FcLangSet *) v.u.l);
-			    break;
-			default:
-			    break;
-			}
 		    }
 		    if (delim != ',')
 			break;

src/fcpat.c

@@ -26,9 +26,6 @@
 #include <string.h>
 #include <assert.h>
 
-static FcBool
-FcHashOwnsName(const FcChar8 *name);
-
 FcPattern *
 FcPatternCreate (void)
 {
@@ -50,7 +47,7 @@ FcValueDestroy (FcValue v)
 {
     switch (v.type) {
     case FcTypeString:
-        if (!FcHashOwnsName(v.u.s))
+	if (!FcSharedStrFree (v.u.s))
             FcStrFree ((FcChar8 *) v.u.s);
 	break;
     case FcTypeMatrix:
@@ -98,7 +95,7 @@ FcValueSave (FcValue v)
 {
     switch (v.type) {
     case FcTypeString:
-	v.u.s = FcStrStaticName (v.u.s);
+	v.u.s = FcSharedStr (v.u.s);
 	if (!v.u.s)
 	    v.type = FcTypeVoid;
 	break;
@@ -131,7 +128,7 @@ FcValueListDestroy (FcValueListPtr l)
     {
 	switch (l->value.type) {
 	case FcTypeString:
-            if (!FcHashOwnsName((FcChar8 *)l->value.u.s))
+	    if (!FcSharedStrFree ((FcChar8 *)l->value.u.s))
                 FcStrFree ((FcChar8 *)l->value.u.s);
 	    break;
 	case FcTypeMatrix:
@@ -652,7 +649,7 @@ FcPatternObjectAddString (FcPattern *p, FcObject object, const FcChar8 *s)
     }
 
     v.type = FcTypeString;
-    v.u.s = FcStrStaticName(s);
+    v.u.s = s;
     return FcPatternObjectAdd (p, object, v, FcTrue);
 }
 
@@ -1030,23 +1027,35 @@ bail0:
 static struct objectBucket {
     struct objectBucket	*next;
     FcChar32		hash;
+    int			ref_count;
 } *FcObjectBuckets[OBJECT_HASH_SIZE];
 
-static FcBool
+FcBool
-FcHashOwnsName (const FcChar8 *name)
+FcSharedStrFree (const FcChar8 *name)
 {
     FcChar32		hash = FcStringHash (name);
     struct objectBucket	**p;
     struct objectBucket	*b;
+    int			size;
 
     for (p = &FcObjectBuckets[hash % OBJECT_HASH_SIZE]; (b = *p); p = &(b->next))
 	if (b->hash == hash && ((char *)name == (char *) (b + 1)))
+	{
+	    b->ref_count--;
+	    if (!b->ref_count)
+	    {
+		*p = b->next;
+		size = sizeof (struct objectBucket) + strlen ((char *)name) + 1;
+		FcMemFree (FC_MEM_SHAREDSTR, size + sizeof (int));
+		free (b);
+	    }
             return FcTrue;
+	}
     return FcFalse;
 }
 
 const FcChar8 *
-FcStrStaticName (const FcChar8 *name)
+FcSharedStr (const FcChar8 *name)
 {
     FcChar32		hash = FcStringHash (name);
     struct objectBucket	**p;
@@ -1055,7 +1064,10 @@ FcStrStaticName (const FcChar8 *name)
 
     for (p = &FcObjectBuckets[hash % OBJECT_HASH_SIZE]; (b = *p); p = &(b->next))
 	if (b->hash == hash && !strcmp ((char *)name, (char *) (b + 1)))
+	{
+	    b->ref_count++;
 	    return (FcChar8 *) (b + 1);
+	}
     size = sizeof (struct objectBucket) + strlen ((char *)name) + 1;
     /*
      * workaround valgrind warning because glibc takes advantage of how it knows memory is
@@ -1063,44 +1075,17 @@ FcStrStaticName (const FcChar8 *name)
      */
     size = (size + 3) & ~3;
     b = malloc (size);
-    FcMemAlloc (FC_MEM_STATICSTR, size);
+    FcMemAlloc (FC_MEM_SHAREDSTR, size);
     if (!b)
         return NULL;
     b->next = 0;
     b->hash = hash;
+    b->ref_count = 1;
     strcpy ((char *) (b + 1), (char *)name);
     *p = b;
     return (FcChar8 *) (b + 1);
 }
 
-static void
-FcStrStaticNameFini (void)
-{
-    int i, size;
-    struct objectBucket *b, *next;
-    char *name;
-
-    for (i = 0; i < OBJECT_HASH_SIZE; i++)
-    {
-	for (b = FcObjectBuckets[i]; b; b = next)
-	{
-	    next = b->next;
-	    name = (char *) (b + 1);
-	    size = sizeof (struct objectBucket) + strlen (name) + 1;
-	    FcMemFree (FC_MEM_STATICSTR, size + sizeof (int));
-	    free (b);
-	}
-	FcObjectBuckets[i] = 0;
-    }
-}
-
-void
-FcPatternFini (void)
-{
-    FcStrStaticNameFini ();
-    FcObjectFini ();
-}
-
 FcBool
 FcPatternSerializeAlloc (FcSerialize *serialize, const FcPattern *pat)
 {

src/fcxml.c

@@ -104,7 +104,7 @@ FcExprCreateString (FcConfig *config, const FcChar8 *s)
     if (e)
     {
 	e->op = FcOpString;
-	e->u.sval = FcStrStaticName (s);
+	e->u.sval = FcSharedStr (s);
     }
     return e;
 }
@@ -176,7 +176,7 @@ FcExprCreateConst (FcConfig *config, const FcChar8 *constant)
     if (e)
     {
 	e->op = FcOpConst;
-	e->u.constant = FcStrStaticName (constant);
+	e->u.constant = FcSharedStr (constant);
     }
     return e;
 }
@@ -205,6 +205,7 @@ FcExprDestroy (FcExpr *e)
     case FcOpDouble:
 	break;
     case FcOpString:
+	FcSharedStrFree (e->u.sval);
 	break;
     case FcOpMatrix:
 	FcMatrixFree (e->u.mval);
@@ -222,6 +223,7 @@ FcExprDestroy (FcExpr *e)
     case FcOpField:
 	break;
     case FcOpConst:
+	FcSharedStrFree (e->u.constant);
 	break;
     case FcOpAssign:
     case FcOpAssignReplace:
@@ -2134,7 +2136,7 @@ FcPopValue (FcConfigParse *parse)
 
     switch (vstack->tag) {
     case FcVStackString:
-	value.u.s = FcStrStaticName (vstack->u.string);
+	value.u.s = FcSharedStr (vstack->u.string);
 	if (value.u.s)
 	    value.type = FcTypeString;
 	break;