tdb: make sure we skip over recovery area correctly.

If it's really the recovery area, we can trust the rec_len field, and
don't have to go groping for bitpatterns.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Autobuild-User: Rusty Russell <rusty@rustcorp.com.au>
Autobuild-Date: Tue Apr 19 14:15:22 CEST 2011 on sn-devel-104

3 files changed, 44 insertions, 17 deletions

diff --git a/lib/tdb/common/summary.c b/lib/tdb/common/summary.c
index b222a5993d7..171a1a20554 100644
--- a/lib/tdb/common/summary.c
+++ b/lib/tdb/common/summary.c
@@ -86,12 +86,13 @@ static size_t get_hash_length(struct tdb_context *tdb, unsigned int i)
 
 _PUBLIC_ char *tdb_summary(struct tdb_context *tdb)
 {
-	tdb_off_t off;
+	tdb_off_t off, rec_off;
 	struct tally freet, keys, data, dead, extra, hash, uncoal;
 	struct tdb_record rec;
 	char *ret = NULL;
 	bool locked;
 	size_t len, unc = 0;
+	struct tdb_record recovery;
 
 	/* Read-only databases use no locking at all: it's best-effort.
 	 * We may have a write lock already, so skip that case too. */
@@ -103,6 +104,10 @@ _PUBLIC_ char *tdb_summary(struct tdb_context *tdb)
 		locked = true;
 	}
 
+	if (tdb_recovery_area(tdb, tdb->methods, &rec_off, &recovery) != 0) {
+		goto unlock;
+	}
+
 	tally_init(&freet);
 	tally_init(&keys);
 	tally_init(&data);
@@ -135,7 +140,11 @@ _PUBLIC_ char *tdb_summary(struct tdb_context *tdb)
 		case TDB_RECOVERY_INVALID_MAGIC:
 		case 0x42424242:
 			unc++;
-			rec.rec_len = tdb_dead_space(tdb, off) - sizeof(rec);
+			/* If it's a valid recovery, we can trust rec_len. */
+			if (off != rec_off) {
+				rec.rec_len = tdb_dead_space(tdb, off)
+					- sizeof(rec);
+			}
 			/* Fall through */
 		case TDB_DEAD_MAGIC:
 			tally_add(&dead, rec.rec_len);
diff --git a/lib/tdb/common/tdb_private.h b/lib/tdb/common/tdb_private.h
index 0186fb9530a..140d4ecec56 100644
--- a/lib/tdb/common/tdb_private.h
+++ b/lib/tdb/common/tdb_private.h
@@ -238,6 +238,10 @@ void tdb_release_transaction_locks(struct tdb_context *tdb);
 int tdb_transaction_lock(struct tdb_context *tdb, int ltype,
 			 enum tdb_lock_flags lockflags);
 int tdb_transaction_unlock(struct tdb_context *tdb, int ltype);
+int tdb_recovery_area(struct tdb_context *tdb,
+		      const struct tdb_methods *methods,
+		      tdb_off_t *recovery_offset,
+		      struct tdb_record *rec);
 int tdb_allrecord_lock(struct tdb_context *tdb, int ltype,
 		       enum tdb_lock_flags flags, bool upgradable);
 int tdb_allrecord_unlock(struct tdb_context *tdb, int ltype, bool mark_lock);
diff --git a/lib/tdb/common/transaction.c b/lib/tdb/common/transaction.c
index e110a08dbcc..e4573cb0a96 100644
--- a/lib/tdb/common/transaction.c
+++ b/lib/tdb/common/transaction.c
@@ -658,6 +658,34 @@ static tdb_len_t tdb_recovery_size(struct tdb_context *tdb)
 	return recovery_size;
 }
 
+int tdb_recovery_area(struct tdb_context *tdb,
+		      const struct tdb_methods *methods,
+		      tdb_off_t *recovery_offset,
+		      struct tdb_record *rec)
+{
+	if (tdb_ofs_read(tdb, TDB_RECOVERY_HEAD, recovery_offset) == -1) {
+		return -1;
+	}
+
+	if (*recovery_offset == 0) {
+		rec->rec_len = 0;
+		return 0;
+	}
+
+	if (methods->tdb_read(tdb, *recovery_offset, rec, sizeof(*rec),
+			      DOCONV()) == -1) {
+		return -1;
+	}
+
+	/* ignore invalid recovery regions: can happen in crash */
+	if (rec->magic != TDB_RECOVERY_MAGIC &&
+	    rec->magic != TDB_RECOVERY_INVALID_MAGIC) {
+		*recovery_offset = 0;
+		rec->rec_len = 0;
+	}
+	return 0;
+}
+
 /*
   allocate the recovery area, or use an existing recovery area if it is
   large enough
@@ -671,25 +699,11 @@ static int tdb_recovery_allocate(struct tdb_context *tdb,
 	const struct tdb_methods *methods = tdb->transaction->io_methods;
 	tdb_off_t recovery_head;
 
-	if (tdb_ofs_read(tdb, TDB_RECOVERY_HEAD, &recovery_head) == -1) {
+	if (tdb_recovery_area(tdb, methods, &recovery_head, &rec) == -1) {
 		TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_recovery_allocate: failed to read recovery head\n"));
 		return -1;
 	}
 
-	rec.rec_len = 0;
-
-	if (recovery_head != 0) {
-		if (methods->tdb_read(tdb, recovery_head, &rec, sizeof(rec), DOCONV()) == -1) {
-			TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_recovery_allocate: failed to read recovery record\n"));
-			return -1;
-		}
-		/* ignore invalid recovery regions: can happen in crash */
-		if (rec.magic != TDB_RECOVERY_MAGIC &&
-		    rec.magic != TDB_RECOVERY_INVALID_MAGIC) {
-			recovery_head = 0;
-		}
-	}
-
 	*recovery_size = tdb_recovery_size(tdb);
 
 	if (recovery_head != 0 && *recovery_size <= rec.rec_len) {