[Fontconfig] fontconfig: Branch 'master'

[Alan Coopersmith]

On 05/30/12 02:34 AM, tagoh at kemper.freedesktop.org wrote:
> 
>     Fix the build fail on Solaris
>     
>     It's introduced by 0ac6c98294d666762960824d39329459b22b48b7.
>     Use lstat() and S_ISDIR() to check if it's the directory or not
>     if there are no d_type in struct dirent.
> 

Sorry, I haven't been paying as much attention to this as I should, but
I did notice this thread and wonder about this fix.

> @@ -163,7 +163,11 @@ Adler32Finish (struct Adler32 *ctx)
>  static FcBool
>  FcDirChecksumScandirFilter(const struct dirent *entry)
>  {
> +#ifdef HAVE_STRUCT_DIRENT_D_TYPE
>      return entry->d_type != DT_DIR;
> +#else
> +    return FcFalse;
> +#endif
>  }

Isn't this backwards?   fontconfig/fontconfig.h has #define FcFalse 0, but at
least the Solaris scandir man page says

     The select argument is a
     pointer to a routine that is called  with  a  pointer  to  a
     directory  entry  and returns a non-zero value if the direc-
     tory entry is included in the  array.

which means this implementation will discard all entries and just waste a bunch
of CPU doing it.   It also says

     If  this  pointer  is NULL, then all the directory entries are included.

which suggests a simpler and more efficient fix of:

--- a/src/fcstat.c
+++ b/src/fcstat.c
@@ -159,16 +159,14 @@ Adler32Finish (struct Adler32 *ctx)
     return ctx->a + (ctx->b << 16);
 }

+#ifdef HAVE_STRUCT_DIRENT_D_TYPE
 /* dirent.d_type can be relied upon on FAT filesystem */
 static FcBool
 FcDirChecksumScandirFilter(const struct dirent *entry)
 {
-#ifdef HAVE_STRUCT_DIRENT_D_TYPE
     return entry->d_type != DT_DIR;
-#else
-    return FcFalse;
-#endif
 }
+#endif

 static int
 FcDirChecksumScandirSorter(const struct dirent **lhs, const struct dirent **rhs
@@ -189,7 +187,11 @@ FcDirChecksum (const FcChar8 *dir, time_t *checksum)
     Adler32Init (&ctx);

     n = scandir ((const char *)dir, &files,
+#ifdef HAVE_STRUCT_DIRENT_D_TYPE
                 &FcDirChecksumScandirFilter,
+#else
+                 NULL /* filter below instead of in scandir */
+#endif
                 &FcDirChecksumScandirSorter);
     if (n == -1)
        return -1;

It also looks like you could save a whole bunch of malloc calls by either using
a fixed size buffer of PATH_MAX, or saving the f pointer and realloc'ing it to
be larger if needed, instead of malloc and free every time, such as:

@@ -184,6 +182,8 @@ FcDirChecksum (const FcChar8 *dir, time_t *checksum)
     int n, ret = 0;
 #ifndef HAVE_STRUCT_DIRENT_D_TYPE
     size_t len = strlen ((const char *)dir);
+    size_t flen = 0;
+    char *f = NULL;
 #endif

     Adler32Init (&ctx);
@@ -203,14 +203,18 @@ FcDirChecksum (const FcChar8 *dir, time_t *checksum)
        dtype = files[n]->d_type;
 #else
        struct stat statb;
-       char *f;
-
-       f = malloc (len + 1 + dlen + 1);
-       if (!f)
-       {
-           ret = -1;
-           goto bail;
-       }
+        int total_len = len + 1 + dlen + 1;
+
+        if (total_len > flen) {
+            char *new_f = realloc(f, total_len);
+            if (!new_f)
+            {
+                free(f);
+                ret = -1;
+                goto bail;
+            }
+            f = new_f;
+        }
        memcpy (f, dir, len);
        f[len] = FC_DIR_SEPARATOR;
        memcpy (&f[len + 1], files[n]->d_name, dlen);
@@ -230,11 +234,12 @@ FcDirChecksum (const FcChar8 *dir, time_t *checksum)

 #ifndef HAVE_STRUCT_DIRENT_D_TYPE
       bail:
-       if (f)
-           free (f);
 #endif
        free (files[n]);
     }
+#ifndef HAVE_STRUCT_DIRENT_D_TYPE
+    free (f);
+#endif
     free (files);
     if (ret == -1)
        return -1;

Both of those are just suggestions which I have not tested (or even tried to
compile).

-- 
	-Alan Coopersmith-              alan.coopersmith at oracle.com
	 Oracle Solaris Engineering - http://blogs.oracle.com/alanc