From 1cb92ea9cf3efcc5f4295b7aeb8ddd10e174127c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 14 Sep 2011 08:13:26 +0930 Subject: 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 (Imported from CCAN commit 1be090a2d749713cfd0c4584cafb97bffd716189) --- lib/tdb2/tdb1_tdb.c | 53 +++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 16 deletions(-) (limited to 'lib/tdb2') 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() */ -- cgit