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

diff --git a/lib/tdb/common/summary.c b/lib/tdb/common/summary.c
index b222a5993d75b75b6a930c4aa10a79c0e7b8b0b2..171a1a205543cf5db64e75571e955082f0681996 100644 (file)
--- 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 0186fb9530a0bb6161038bcc6c2193aafa401dfd..140d4ecec5695660e36f7cdcec447c7283296c25 100644 (file)
--- 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 e110a08dbcca91ac580efaf08524510fa5006402..e4573cb0a96113cc0d9cb3dcb5eafdebf56f0e82 100644 (file)
--- 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) {