On 05/30/12 02:34 AM, tagoh@xxxxxxxxxxxxxxxxxxxxxx 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@xxxxxxxxxx
Oracle Solaris Engineering - http://blogs.oracle.com/alanc
_______________________________________________
Fontconfig mailing list
Fontconfig@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/fontconfig
