Re: Repeat recovers on databases

[Bron Gondwana <brong@xxxxxxxxxxx>]

On Thu, Jun 18, 2009 at 05:44:19PM -0400, Michael Bacon wrote:
> The fix for it is below.  I will also open a bugzilla issue for this.

I think this is actually a better fix that keeps things in the
right type on to disk.  Can you please test it on your platform.

Thanks,

Bron.

>From 4adb3fb5d7f2085596d393f56beda6cf4236dd23 Mon Sep 17 00:00:00 2001
From: Bron Gondwana <brong@xxxxxxxxxxx>
Date: Fri, 19 Jun 2009 09:59:45 +1000
Subject: [PATCH] Use correctly sized variable for recovery time

Global Recovery was being htonl'd into a time_t,
which is a 64 bit value on some platforms.

Depending on the host bit order, this might be
promoted into the last 4 bytes of an 8 bit value,
meaning that the bytes written to disk were actually
all zero.

Change it to a bit32 and give it a more descriptive
name.

Thanks to Michael Bacon <baconm@xxxxxxxxxxxxx> for
finding the bug.
---
 lib/cyrusdb_skiplist.c |   34 +++++++++++++++++-----------------
 1 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/lib/cyrusdb_skiplist.c b/lib/cyrusdb_skiplist.c
index 0ab46ba..aac1cd1 100644
--- a/lib/cyrusdb_skiplist.c
+++ b/lib/cyrusdb_skiplist.c
@@ -192,6 +192,18 @@ struct db_list {
 static time_t global_recovery = 0;
 static struct db_list *open_db = NULL;
 
+#define BIT32_MAX 4294967295U
+
+#if UINT_MAX == BIT32_MAX
+typedef unsigned int bit32;
+#elif ULONG_MAX == BIT32_MAX
+typedef unsigned long bit32;
+#elif USHRT_MAX == BIT32_MAX
+typedef unsigned short bit32;
+#else
+#error dont know what to use for bit32
+#endif
+
 /* Perform an FSYNC/FDATASYNC if we are *not* operating in UNSAFE mode */
 #define DO_FSYNC (!libcyrus_config_getswitch(CYRUSOPT_SKIPLIST_UNSAFE))
 
@@ -226,7 +238,7 @@ static int myinit(const char *dbdir, int myflags)
 {
     char sfile[1024];
     int fd, r = 0;
-    time_t a;
+    bit32 net32_time;
     
     snprintf(sfile, sizeof(sfile), "%s/skipstamp", dbdir);
 
@@ -239,8 +251,8 @@ static int myinit(const char *dbdir, int myflags)
 	if (fd == -1) r = -1;
 
 	if (r != -1) r = ftruncate(fd, 0);
-	a = htonl(global_recovery);
-	if (r != -1) r = write(fd, &a, 4);
+	net32_time = htonl(global_recovery);
+	if (r != -1) r = write(fd, &net32_time, 4);
 	if (r != -1) r = close(fd);
 
 	if (r == -1) {
@@ -253,7 +265,7 @@ static int myinit(const char *dbdir, int myflags)
 
 	fd = open(sfile, O_RDONLY, 0644);
 	if (fd == -1) r = -1;
-	if (r != -1) r = read(fd, &a, 4);
+	if (r != -1) r = read(fd, &net32_time, 4);
 	if (r != -1) r = close(fd);
 
 	if (r == -1) {
@@ -261,7 +273,7 @@ static int myinit(const char *dbdir, int myflags)
 		   sfile);
 	    global_recovery = 0;
 	} else {
-	    global_recovery = ntohl(a);
+	    global_recovery = ntohl(net32_time);
 	}
     }
 
@@ -316,18 +328,6 @@ enum {
     SKIPLIST_MINREWRITE = 4096 /* don't rewrite logs smaller than this */
 };
 
-#define BIT32_MAX 4294967295U
-
-#if UINT_MAX == BIT32_MAX
-typedef unsigned int bit32;
-#elif ULONG_MAX == BIT32_MAX
-typedef unsigned long bit32;
-#elif USHRT_MAX == BIT32_MAX
-typedef unsigned short bit32;
-#else
-#error dont know what to use for bit32
-#endif
-
 #define HEADER_MAGIC ("\241\002\213\015skiplist file\0\0\0")
 #define HEADER_MAGIC_SIZE (20)
 
-- 
1.6.2.2.446.gfbdc0

----
Cyrus Home Page: http://cyrusimap.web.cmu.edu/
Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki
List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html