Attachment 43671 Details for Bug 25535 – Fix cache aging for fonts on FAT [v2]

[patch] Fix cache aging for fonts on FAT [v2]

0001-Fix-cache-aging-for-fonts-on-FAT-filesystem-under-Li.patch (text/plain), 14.57 KB, created by Mikhail Gusarov on 2011-02-22 10:18:44 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Mikhail Gusarov

Created: 2011-02-22 10:18:44 UTC

Size: 14.57 KB

patch

obsolete

>From b85db374f5da8c7394b8b02b791f528853d4c7fd Mon Sep 17 00:00:00 2001
>From: Mikhail Gusarov <dottedmag@dottedmag.net>
>Date: Tue, 22 Feb 2011 19:06:42 +0100
>Subject: [PATCH 1/2] Fix cache aging for fonts on FAT filesystem under Linux
>
>Windows does not update mtime of directory on FAT filesystem when
>file is added to it or removed from it. Fontconfig uses mtime of
>directory to check cache file aging and hence fails to detect
>newly added or recently removed files.
>
>This patch detects FAT filesystem (currently implemented for
>Linux) and adds generating checksum of directory entries instead
>of using mtime which guarantees proper cache rebuild.
>
>For non-FAT filesystems this patch adds single syscall per directory
>which is negligeable overhead.
>
>This fixes bug https://bugs.freedesktop.org/show_bug.cgi?id=25535
>
>This is version 2 of patch. Changes since version 1:
>
>  - Fixed memory leak with scandir(3) results not being freed
>  - Implemented stable sorting for scandir(3) not using strcoll(3).
>  - FcStat and FcStatChecksum moved to separate fcstat.c
>  - Changed FcDirChecksum signature to avoid weakening Adler-32 checksum in
>    order to be able to return error from function.
>
>Signed-off-by: Mikhail Gusarov <dottedmag@dottedmag.net>
>---
> src/Makefile.am |    1 +
> src/fccache.c   |   93 ++---------------------
> src/fcdir.c     |    2 +-
> src/fcint.h     |   25 ++++--
> src/fcstat.c    |  230 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 256 insertions(+), 95 deletions(-)
> create mode 100644 src/fcstat.c
>
>diff --git a/src/Makefile.am b/src/Makefile.am
>index 090f9b1..df13329 100644
>--- a/src/Makefile.am
>+++ b/src/Makefile.am
>@@ -118,6 +118,7 @@ libfontconfig_la_SOURCES = \
> 	fcname.c \
> 	fcpat.c \
> 	fcserialize.c \
>+	fcstat.c \
> 	fcstr.c \
> 	fcxml.c \
> 	ftglue.h \
>diff --git a/src/fccache.c b/src/fccache.c
>index 1bbd799..23e344a 100644
>--- a/src/fccache.c
>+++ b/src/fccache.c
>@@ -25,7 +25,6 @@
> #include "fcarch.h"
> #include <stdio.h>
> #include <fcntl.h>
>-#include <dirent.h>
> #include <string.h>
> #include <sys/types.h>
> #include <assert.h>
>@@ -55,84 +54,6 @@ static void MD5Transform(FcChar32 buf[4], FcChar32 in[16]);
> 
> #define CACHEBASE_LEN (1 + 32 + 1 + sizeof (FC_ARCHITECTURE) + sizeof (FC_CACHE_SUFFIX))
> 
>-#ifdef _WIN32
>-
>-#include <windows.h>
>-
>-#ifdef __GNUC__
>-typedef long long INT64;
>-#define EPOCH_OFFSET 11644473600ll
>-#else
>-#define EPOCH_OFFSET 11644473600i64
>-typedef __int64 INT64;
>-#endif
>-
>-/* Workaround for problems in the stat() in the Microsoft C library:
>- *
>- * 1) stat() uses FindFirstFile() to get the file
>- * attributes. Unfortunately this API doesn't return correct values
>- * for modification time of a directory until some time after a file
>- * or subdirectory has been added to the directory. (This causes
>- * run-test.sh to fail, for instance.) GetFileAttributesEx() is
>- * better, it returns the updated timestamp right away.
>- *
>- * 2) stat() does some strange things related to backward
>- * compatibility with the local time timestamps on FAT volumes and
>- * daylight saving time. This causes problems after the switches
>- * to/from daylight saving time. See
>- * http://bugzilla.gnome.org/show_bug.cgi?id=154968 , especially
>- * comment #30, and http://www.codeproject.com/datetime/dstbugs.asp .
>- * We don't need any of that, FAT and Win9x are as good as dead. So
>- * just use the UTC timestamps from NTFS, converted to the Unix epoch.
>- */
>-
>-int
>-FcStat (const char *file, struct stat *statb)
>-{
>-    WIN32_FILE_ATTRIBUTE_DATA wfad;
>-    char full_path_name[MAX_PATH];
>-    char *basename;
>-    DWORD rc;
>-
>-    if (!GetFileAttributesEx (file, GetFileExInfoStandard, &wfad))
>-	return -1;
>-
>-    statb->st_dev = 0;
>-
>-    /* Calculate a pseudo inode number as a hash of the full path name.
>-     * Call GetLongPathName() to get the spelling of the path name as it
>-     * is on disk.
>-     */
>-    rc = GetFullPathName (file, sizeof (full_path_name), full_path_name, &basename);
>-    if (rc == 0 || rc > sizeof (full_path_name))
>-	return -1;
>-
>-    rc = GetLongPathName (full_path_name, full_path_name, sizeof (full_path_name));
>-    statb->st_ino = FcStringHash (full_path_name);
>-
>-    statb->st_mode = _S_IREAD | _S_IWRITE;
>-    statb->st_mode |= (statb->st_mode >> 3) | (statb->st_mode >> 6);
>-
>-    if (wfad.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
>-	statb->st_mode |= _S_IFDIR;
>-    else
>-	statb->st_mode |= _S_IFREG;
>-
>-    statb->st_nlink = 1;
>-    statb->st_uid = statb->st_gid = 0;
>-    statb->st_rdev = 0;
>-
>-    if (wfad.nFileSizeHigh > 0)
>-	return -1;
>-    statb->st_size = wfad.nFileSizeLow;
>-
>-    statb->st_atime = (*(INT64 *)&wfad.ftLastAccessTime)/10000000 - EPOCH_OFFSET;
>-    statb->st_mtime = (*(INT64 *)&wfad.ftLastWriteTime)/10000000 - EPOCH_OFFSET;
>-    statb->st_ctime = statb->st_mtime;
>-
>-    return 0;
>-}
>-#endif
> 
> static const char bin2hex[] = { '0', '1', '2', '3',
> 				'4', '5', '6', '7',
>@@ -234,7 +155,7 @@ FcDirCacheProcess (FcConfig *config, const FcChar8 *dir,
>     struct stat file_stat, dir_stat;
>     FcBool	ret = FcFalse;
> 
>-    if (FcStat ((char *) dir, &dir_stat) < 0)
>+    if (FcStatChecksum ((char *) dir, &dir_stat) < 0)
>         return FcFalse;
> 
>     FcDirCacheBasename (dir, cache_base);
>@@ -516,14 +437,14 @@ FcCacheTimeValid (FcCache *cache, struct stat *dir_stat)
> 
>     if (!dir_stat)
>     {
>-	if (FcStat ((const char *) FcCacheDir (cache), &dir_static) < 0)
>+	if (FcStatChecksum ((const char *) FcCacheDir (cache), &dir_static) < 0)
> 	    return FcFalse;
> 	dir_stat = &dir_static;
>     }
>     if (FcDebug () & FC_DBG_CACHE)
>-	printf ("FcCacheTimeValid dir \"%s\" cache time %d dir time %d\n",
>-		FcCacheDir (cache), cache->mtime, (int) dir_stat->st_mtime);
>-    return cache->mtime == (int) dir_stat->st_mtime;
>+	printf ("FcCacheTimeValid dir \"%s\" cache checksum %d dir checksum %d\n",
>+		FcCacheDir (cache), cache->checksum, (int) dir_stat->st_mtime);
>+    return cache->checksum == (int) dir_stat->st_mtime;
> }
> 
> /*
>@@ -684,7 +605,7 @@ FcDirCacheValidateHelper (int fd, struct stat *fd_stat, struct stat *dir_stat, v
> 	ret = FcFalse;
>     else if (fd_stat->st_size != c.size)
> 	ret = FcFalse;
>-    else if (c.mtime != (int) dir_stat->st_mtime)
>+    else if (c.checksum != (int) dir_stat->st_mtime)
> 	ret = FcFalse;
>     return ret;
> }
>@@ -761,7 +682,7 @@ FcDirCacheBuild (FcFontSet *set, const FcChar8 *dir, struct stat *dir_stat, FcSt
>     cache->magic = FC_CACHE_MAGIC_ALLOC;
>     cache->version = FC_CACHE_CONTENT_VERSION;
>     cache->size = serialize->size;
>-    cache->mtime = (int) dir_stat->st_mtime;
>+    cache->checksum = (int) dir_stat->st_mtime;
> 
>     /*
>      * Serialize directory name
>diff --git a/src/fcdir.c b/src/fcdir.c
>index e6b66ce..fbf05ae 100644
>--- a/src/fcdir.c
>+++ b/src/fcdir.c
>@@ -243,7 +243,7 @@ FcDirCacheScan (const FcChar8 *dir, FcConfig *config)
>     if (FcDebug () & FC_DBG_FONTSET)
> 	printf ("cache scan dir %s\n", dir);
> 
>-    if (FcStat ((char *) dir, &dir_stat) < 0)
>+    if (FcStatChecksum ((char *) dir, &dir_stat) < 0)
>     {
> 	if (errno != ENOENT)
> 	    ret = FcFalse;
>diff --git a/src/fcint.h b/src/fcint.h
>index e662c37..1fdc19c 100644
>--- a/src/fcint.h
>+++ b/src/fcint.h
>@@ -338,7 +338,7 @@ struct _FcCache {
>     intptr_t	dirs;		    /* offset to subdirs */
>     int		dirs_count;	    /* number of subdir strings */
>     intptr_t	set;		    /* offset to font set */
>-    int		mtime;		    /* low bits of directory mtime */
>+    int		checksum;		    /* checksum of directory state */
> };
> 
> #undef FcCacheDir
>@@ -544,13 +544,6 @@ FcCacheFini (void);
> FcPrivate void
> FcDirCacheReference (FcCache *cache, int nref);
> 
>-#ifdef _WIN32
>-FcPrivate int
>-FcStat (const char *file, struct stat *statb);
>-#else
>-#define FcStat stat
>-#endif
>-
> /* fccfg.c */
> 
> FcPrivate FcExpr *
>@@ -1040,4 +1033,20 @@ FcStrSerializeAlloc (FcSerialize *serialize, const FcChar8 *str);
> FcPrivate FcChar8 *
> FcStrSerialize (FcSerialize *serialize, const FcChar8 *str);
> 
>+/* fcstat.c */
>+
>+#ifdef _WIN32
>+FcPrivate int
>+FcStat (const char *file, struct stat *statb);
>+#else
>+#define FcStat stat
>+#endif
>+
>+#ifdef linux
>+FcPrivate int
>+FcStatChecksum (const char *file, struct stat *statb);
>+#else
>+#define FcStatChecksum FcStat
>+#endif
>+
> #endif /* _FC_INT_H_ */
>diff --git a/src/fcstat.c b/src/fcstat.c
>new file mode 100644
>index 0000000..cb5a61a
>--- /dev/null
>+++ b/src/fcstat.c
>@@ -0,0 +1,230 @@
>+/*
>+ * Copyright Â© 2000 Keith Packard
>+ * Copyright Â© 2005 Patrick Lam
>+ * Copyright Â© 2011 Mikhail Gusarov
>+ *
>+ * Permission to use, copy, modify, distribute, and sell this software and its
>+ * documentation for any purpose is hereby granted without fee, provided that
>+ * the above copyright notice appear in all copies and that both that
>+ * copyright notice and this permission notice appear in supporting
>+ * documentation, and that the name of the author(s) not be used in
>+ * advertising or publicity pertaining to distribution of the software without
>+ * specific, written prior permission.  The authors make no
>+ * representations about the suitability of this software for any purpose.  It
>+ * is provided "as is" without express or implied warranty.
>+ *
>+ * THE AUTHOR(S) DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
>+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
>+ * EVENT SHALL THE AUTHOR(S) BE LIABLE FOR ANY SPECIAL, INDIRECT OR
>+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
>+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
>+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
>+ * PERFORMANCE OF THIS SOFTWARE.
>+ */
>+
>+#include "fcint.h"
>+#include <dirent.h>
>+/* For FcFsMtimeBroken */
>+#if defined(linux)
>+#  include <sys/vfs.h>
>+#  include <linux/magic.h>
>+#endif
>+
>+#ifdef _WIN32
>+
>+#include <windows.h>
>+
>+#ifdef __GNUC__
>+typedef long long INT64;
>+#define EPOCH_OFFSET 11644473600ll
>+#else
>+#define EPOCH_OFFSET 11644473600i64
>+typedef __int64 INT64;
>+#endif
>+
>+/* Workaround for problems in the stat() in the Microsoft C library:
>+ *
>+ * 1) stat() uses FindFirstFile() to get the file
>+ * attributes. Unfortunately this API doesn't return correct values
>+ * for modification time of a directory until some time after a file
>+ * or subdirectory has been added to the directory. (This causes
>+ * run-test.sh to fail, for instance.) GetFileAttributesEx() is
>+ * better, it returns the updated timestamp right away.
>+ *
>+ * 2) stat() does some strange things related to backward
>+ * compatibility with the local time timestamps on FAT volumes and
>+ * daylight saving time. This causes problems after the switches
>+ * to/from daylight saving time. See
>+ * http://bugzilla.gnome.org/show_bug.cgi?id=154968 , especially
>+ * comment #30, and http://www.codeproject.com/datetime/dstbugs.asp .
>+ * We don't need any of that, FAT and Win9x are as good as dead. So
>+ * just use the UTC timestamps from NTFS, converted to the Unix epoch.
>+ */
>+
>+int
>+FcStat (const char *file, struct stat *statb)
>+{
>+    WIN32_FILE_ATTRIBUTE_DATA wfad;
>+    char full_path_name[MAX_PATH];
>+    char *basename;
>+    DWORD rc;
>+
>+    if (!GetFileAttributesEx (file, GetFileExInfoStandard, &wfad))
>+	return -1;
>+
>+    statb->st_dev = 0;
>+
>+    /* Calculate a pseudo inode number as a hash of the full path name.
>+     * Call GetLongPathName() to get the spelling of the path name as it
>+     * is on disk.
>+     */
>+    rc = GetFullPathName (file, sizeof (full_path_name), full_path_name, &basename);
>+    if (rc == 0 || rc > sizeof (full_path_name))
>+	return -1;
>+
>+    rc = GetLongPathName (full_path_name, full_path_name, sizeof (full_path_name));
>+    statb->st_ino = FcStringHash (full_path_name);
>+
>+    statb->st_mode = _S_IREAD | _S_IWRITE;
>+    statb->st_mode |= (statb->st_mode >> 3) | (statb->st_mode >> 6);
>+
>+    if (wfad.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
>+	statb->st_mode |= _S_IFDIR;
>+    else
>+	statb->st_mode |= _S_IFREG;
>+
>+    statb->st_nlink = 1;
>+    statb->st_uid = statb->st_gid = 0;
>+    statb->st_rdev = 0;
>+
>+    if (wfad.nFileSizeHigh > 0)
>+	return -1;
>+    statb->st_size = wfad.nFileSizeLow;
>+
>+    statb->st_atime = (*(INT64 *)&wfad.ftLastAccessTime)/10000000 - EPOCH_OFFSET;
>+    statb->st_mtime = (*(INT64 *)&wfad.ftLastWriteTime)/10000000 - EPOCH_OFFSET;
>+    statb->st_ctime = statb->st_mtime;
>+
>+    return 0;
>+}
>+#endif
>+
>+/*
>+ * Checking for FAT filesystem which broken mtime handling. There is no
>+ * cross-platform filesystem type query call, so resort to OS-specific calls.
>+ */
>+#if defined(linux)
>+static FcBool
>+FcFsMtimeBroken (const char *file)
>+{
>+    int ret;
>+    struct statfs fs_stat;
>+
>+    if (FcDebug () & FC_DBG_CACHE)
>+	printf ("FcFsMtimeBroken %s\n", file);
>+
>+    ret = statfs (file, &fs_stat);
>+    if (ret == -1)
>+	return FcFalse;
>+
>+    if (FcDebug () & FC_DBG_CACHE)
>+	printf ("FcFsMtimeBroken f_type %ld MSDOS_SUPER_MAGIC %d\n",
>+	        fs_stat.f_type, MSDOS_SUPER_MAGIC);
>+
>+    if (fs_stat.f_type == MSDOS_SUPER_MAGIC)
>+	return FcTrue;
>+
>+    return FcFalse;
>+}
>+#endif
>+
>+/*
>+ * This is to be enabled on all systems which support checking for FAT fs, not
>+ * just Linux, so change this #if after updating FcFsMtimeBroken.
>+ */
>+#if defined(linux)
>+
>+/* Adler-32 checksum implementation */
>+struct Adler32 {
>+    int a;
>+    int b;
>+};
>+
>+static void
>+Adler32Init (struct Adler32 *ctx)
>+{
>+    ctx->a = 1;
>+    ctx->b = 0;
>+}
>+
>+static void
>+Adler32Update (struct Adler32 *ctx, const char *data, int data_len)
>+{
>+    while (data_len--)
>+    {
>+	ctx->a = (ctx->a + *data++) % 65521;
>+	ctx->b = (ctx->b + ctx->a) % 65521;
>+    }
>+}
>+
>+static int
>+Adler32Finish (struct Adler32 *ctx)
>+{
>+    return ctx->a + (ctx->b << 16);
>+}
>+
>+/* dirent.d_type can be relied upon on FAT filesystem */
>+static FcBool
>+FcDirChecksumScandirFilter(const struct dirent *entry)
>+{
>+    return entry->d_type != DT_DIR;
>+}
>+
>+static int
>+FcDirChecksumScandirSorter(const struct dirent **lhs, const struct dirent **rhs)
>+{
>+    return strcmp((*lhs)->d_name, (*rhs)->d_name);
>+}
>+
>+static int
>+FcDirChecksum (const char *dir, int *checksum)
>+{
>+    struct Adler32 ctx;
>+    struct dirent **files;
>+    int n;
>+
>+    Adler32Init (&ctx);
>+
>+    n = scandir (dir, &files,
>+                 &FcDirChecksumScandirFilter,
>+                 &FcDirChecksumScandirSorter);
>+    if (n == -1)
>+        return -1;
>+
>+    while (n--)
>+    {
>+        Adler32Update (&ctx, files[n]->d_name, strlen(files[n]->d_name) + 1);
>+        Adler32Update (&ctx, (char *)&files[n]->d_type, sizeof(files[n]->d_type));
>+        free(files[n]);
>+    }
>+    free(files);
>+
>+    *checksum = Adler32Finish (&ctx);
>+    return 0;
>+}
>+
>+int
>+FcStatChecksum (const char *file, struct stat *statb)
>+{
>+    if (FcStat (file, statb) == -1)
>+        return -1;
>+
>+    if (FcFsMtimeBroken (file))
>+    {
>+        if (FcDirChecksum (file, &statb->st_mtime) == -1)
>+            return -1;
>+    }
>+
>+    return 0;
>+}
>+#endif
>-- 
>1.7.2.3
>

Actions: View | Splinter Review

Attachments on bug 25535: 32209 | 43671 | 43672 | 60763 | 60764 | 61532 | 61533 | 61534

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.