tdb2: don't continue if tdb1_find fails.

The TDB1 code's tdb1_find() returns 0 on error; the callers should
not assume that the error means that the entry wasn't found, but use
last_error to determine it.

This was found by looking at how long the failure path testing for
test/run-10-simple-store.c was taking under valgrind, ie:

	valgrind -q ./run-10-simple-store --show-slowest

This change dropped the time for that test from 53 seconds to 19
seconds.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
(Imported from CCAN commit 1be090a2d749713cfd0c4584cafb97bffd716189)

1 files changed, 37 insertions, 16 deletions

diff --git a/lib/tdb2/tdb1_tdb.c b/lib/tdb2/tdb1_tdb.c
index 45db2ba33b..6f6f080da2 100644
--- a/lib/tdb2/tdb1_tdb.c
+++ b/lib/tdb2/tdb1_tdb.c
@@ -68,13 +68,17 @@ static void tdb1_increment_seqnum(struct tdb_context *tdb)
 	tdb1_nest_unlock(tdb, TDB1_SEQNUM_OFS, F_WRLCK);
 }
 
-static int tdb1_key_compare(TDB_DATA key, TDB_DATA data, void *private_data)
+static enum TDB_ERROR tdb1_key_compare(TDB_DATA key, TDB_DATA data,
+				       void *matches_)
 {
-	return memcmp(data.dptr, key.dptr, data.dsize);
+	bool *matches = matches_;
+	*matches = (memcmp(data.dptr, key.dptr, data.dsize) == 0);
+	return TDB_SUCCESS;
 }
 
-/* Returns 0 on fail.  On success, return offset of record, and fills
-   in rec */
+/* Returns 0 on fail; last_error will be TDB_ERR_NOEXIST if it simply
+ * wasn't there, otherwise a real error.
+ * On success, return offset of record, and fills in rec */
 static tdb1_off_t tdb1_find(struct tdb_context *tdb, TDB_DATA key, uint32_t hash,
 			struct tdb1_record *r)
 {
@@ -96,12 +100,23 @@ static tdb1_off_t tdb1_find(struct tdb_context *tdb, TDB_DATA key, uint32_t hash
 			tdb->stats.compare_wrong_keylen++;
 		} else if (hash != r->full_hash) {
 			tdb->stats.compare_wrong_rechash++;
-		} else if (tdb1_parse_data(tdb, key, rec_ptr + sizeof(*r),
-					   r->key_len, tdb1_key_compare,
-					   NULL) != 0) {
-			tdb->stats.compare_wrong_keycmp++;
 		} else {
-			return rec_ptr;
+			enum TDB_ERROR ecode;
+			bool matches;
+			ecode = tdb1_parse_data(tdb, key, rec_ptr + sizeof(*r),
+						r->key_len, tdb1_key_compare,
+						&matches);
+
+			if (ecode != TDB_SUCCESS) {
+				tdb->last_error = ecode;
+				return 0;
+			}
+
+			if (!matches) {
+				tdb->stats.compare_wrong_keycmp++;
+			} else {
+				return rec_ptr;
+			}
 		}
 		/* detect tight infinite loop */
 		if (rec_ptr == r->next) {
@@ -232,8 +247,7 @@ enum TDB_ERROR tdb1_parse_record(struct tdb_context *tdb, TDB_DATA key,
 	hash = tdb_hash(tdb, key.dptr, key.dsize);
 
 	if (!(rec_ptr = tdb1_find_lock_hash(tdb,key,hash,F_RDLCK,&rec))) {
-		/* record not found */
-		return TDB_ERR_NOEXIST;
+		return tdb->last_error;
 	}
 
 	ret = tdb1_parse_data(tdb, key, rec_ptr + sizeof(rec) + rec.key_len,
@@ -473,16 +487,23 @@ static int _tdb1_store(struct tdb_context *tdb, TDB_DATA key,
 			tdb->last_error = TDB_ERR_EXISTS;
 			goto fail;
 		}
+		if (tdb->last_error != TDB_ERR_NOEXIST) {
+			goto fail;
+		}
 	} else {
 		/* first try in-place update, on modify or replace. */
 		if (tdb1_update_hash(tdb, key, hash, dbuf) == 0) {
 			goto done;
 		}
-		if (tdb->last_error == TDB_ERR_NOEXIST &&
-		    flag == TDB_MODIFY) {
-			/* if the record doesn't exist and we are in TDB1_MODIFY mode then
-			 we should fail the store */
-			goto fail;
+		if (tdb->last_error != TDB_SUCCESS) {
+			if (tdb->last_error != TDB_ERR_NOEXIST) {
+				goto fail;
+			}
+			if (flag == TDB_MODIFY) {
+				/* if the record doesn't exist and we are in TDB1_MODIFY mode then
+				   we should fail the store */
+				goto fail;
+			}
 		}
 	}
 	/* reset the error code potentially set by the tdb1_update() */