[Fontconfig] fontconfig: Branch 'master' - 2 commits

Akira TAGOH tagoh at kemper.freedesktop.org
Tue Mar 27 21:44:09 PDT 2012


 src/fccfg.c  |    2 -
 src/fcinit.c |    4 +--
 src/fcint.h  |   10 ++++-----
 src/fclist.c |   10 ++++++++-
 src/fcname.c |   34 +++++++-----------------------
 src/fcpat.c  |   65 ++++++++++++++++++++++-------------------------------------
 src/fcxml.c  |    8 ++++---
 7 files changed, 55 insertions(+), 78 deletions(-)

New commits:
commit 4a060729a1466186d3be63ada344f43d66f937e5
Author: Akira TAGOH <akira at tagoh.org>
Date:   Wed Mar 28 13:38:53 2012 +0900

    fcpat: Increase the number of buckets in the shared string hash table
    
    This is a reasonably conservative increase in the number of buckets in the hash
    table to 251.  After FcInit(), there are 240 shared strings in use on my system
    (from configuration files I assume).  The hash value is stored in each link in
    the chains so comparison are actually not very expensive.  This change should
    reduce the average length of chains by a factor of 8.  With the reference
    counted strings, it should keep the average length of chains to about 2.  The
    number of buckets is prime so as not to rely too much on the quality of the
    hash function.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=17832#c5
    
    Patch from Karl Tomlinson

diff --git a/src/fcpat.c b/src/fcpat.c
index 4b93c5e..54ec45c 100644
--- a/src/fcpat.c
+++ b/src/fcpat.c
@@ -1023,7 +1023,7 @@ bail0:
     return NULL;
 }
 
-#define OBJECT_HASH_SIZE    31
+#define OBJECT_HASH_SIZE    251
 static struct objectBucket {
     struct objectBucket	*next;
     FcChar32		hash;
commit d8dcff7b96b09748e6f1df9e4adc7ab0850d7b18
Author: Akira TAGOH <akira at tagoh.org>
Date:   Wed Mar 28 13:37:15 2012 +0900

    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

diff --git a/src/fccfg.c b/src/fccfg.c
index 9395f74..bd1dc34 100644
--- a/src/fccfg.c
+++ b/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)
diff --git a/src/fcinit.c b/src/fcinit.c
index b7966b6..abf64b5 100644
--- a/src/fcinit.c
+++ b/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;
diff --git a/src/fcint.h b/src/fcint.h
index 8179195..56f77ef 100644
--- a/src/fcint.h
+++ b/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);
diff --git a/src/fclist.c b/src/fclist.c
index 9a84b5c..88025e9 100644
--- a/src/fclist.c
+++ b/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);
     }
diff --git a/src/fcname.c b/src/fcname.c
index 1b32b0f..d0b1ca8 100644
--- a/src/fcname.c
+++ b/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) {
-			    case FcTypeCharSet:
-				FcCharSetDestroy ((FcCharSet *) v.u.c);
-				break;
-			    case FcTypeLangSet:
-				FcLangSetDestroy ((FcLangSet *) v.u.l);
-				break;
-			    default:
-				break;
-			    }
+			    FcValueDestroy (v);
 			    goto bail2;
 			}
-			switch (v.type) {
-			case FcTypeCharSet:
-			    FcCharSetDestroy ((FcCharSet *) v.u.c);
-			    break;
-			case FcTypeLangSet:
-			    FcLangSetDestroy ((FcLangSet *) v.u.l);
-			    break;
-			default:
-			    break;
-			}
+			FcValueDestroy (v);
 		    }
 		    if (delim != ',')
 			break;
diff --git a/src/fcpat.c b/src/fcpat.c
index 8f63659..4b93c5e 100644
--- a/src/fcpat.c
+++ b/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
-FcHashOwnsName (const FcChar8 *name)
+FcBool
+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)
 {
diff --git a/src/fcxml.c b/src/fcxml.c
index ff30b7b..0fb82b6 100644
--- a/src/fcxml.c
+++ b/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;


More information about the Fontconfig mailing list