summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRusty Russell <rusty@rustcorp.com.au>2011-09-14 08:13:26 +0930
committerRusty Russell <rusty@rustcorp.com.au>2011-09-14 08:13:26 +0930
commit1cb92ea9cf3efcc5f4295b7aeb8ddd10e174127c (patch)
tree01c94f07d81359ecc1d452864bfcbc8ab4d2908a
parent01b2214a1893db5071addf1fdf17e9ac06ed63a0 (diff)
downloadsamba-1cb92ea9cf3efcc5f4295b7aeb8ddd10e174127c.tar.gz
samba-1cb92ea9cf3efcc5f4295b7aeb8ddd10e174127c.tar.bz2
samba-1cb92ea9cf3efcc5f4295b7aeb8ddd10e174127c.zip
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)
-rw-r--r--lib/tdb2/tdb1_tdb.c53
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() */