From 3835bb95d716cb6448913792b0d8d539877d93e9 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 1 Mar 2011 23:19:19 +1030 Subject: [PATCH] tdb2: rework free.c functions to return enum TDB_ERROR. --- ccan/tdb2/free.c | 235 ++++++++++++++------------- ccan/tdb2/hash.c | 8 +- ccan/tdb2/private.h | 17 +- ccan/tdb2/tdb.c | 47 ++++-- ccan/tdb2/test/run-01-new_database.c | 4 +- ccan/tdb2/test/run-04-basichash.c | 4 +- ccan/tdb2/transaction.c | 7 +- 7 files changed, 169 insertions(+), 153 deletions(-) diff --git a/ccan/tdb2/free.c b/ccan/tdb2/free.c index 897d3cbe..97320b58 100644 --- a/ccan/tdb2/free.c +++ b/ccan/tdb2/free.c @@ -59,7 +59,7 @@ tdb_off_t next_ftable(struct tdb_context *tdb, tdb_off_t ftable) return tdb_read_off(tdb, ftable + offsetof(struct tdb_freetable,next)); } -int tdb_ftable_init(struct tdb_context *tdb) +enum TDB_ERROR tdb_ftable_init(struct tdb_context *tdb) { /* Use reservoir sampling algorithm to select a free list at random. */ unsigned int rnd, max = 0, count = 0; @@ -70,8 +70,7 @@ int tdb_ftable_init(struct tdb_context *tdb) while (off) { if (TDB_OFF_IS_ERR(off)) { - tdb->ecode = off; - return -1; + return off; } rnd = random(); @@ -84,7 +83,7 @@ int tdb_ftable_init(struct tdb_context *tdb) off = next_ftable(tdb, off); count++; } - return 0; + return TDB_SUCCESS; } /* Offset of a given bucket. */ @@ -105,9 +104,9 @@ static tdb_off_t find_free_head(struct tdb_context *tdb, } /* Remove from free bucket. */ -static int remove_from_list(struct tdb_context *tdb, - tdb_off_t b_off, tdb_off_t r_off, - const struct tdb_free_record *r) +static enum TDB_ERROR remove_from_list(struct tdb_context *tdb, + tdb_off_t b_off, tdb_off_t r_off, + const struct tdb_free_record *r) { tdb_off_t off; enum TDB_ERROR ecode; @@ -121,18 +120,17 @@ static int remove_from_list(struct tdb_context *tdb, #ifdef CCAN_TDB2_DEBUG if (tdb_read_off(tdb, off) != r_off) { - tdb_logerr(tdb, TDB_ERR_CORRUPT, TDB_LOG_ERROR, - "remove_from_list: %llu bad prev in list %llu", - (long long)r_off, (long long)b_off); - return -1; + return tdb_logerr(tdb, TDB_ERR_CORRUPT, TDB_LOG_ERROR, + "remove_from_list:" + " %llu bad prev in list %llu", + (long long)r_off, (long long)b_off); } #endif /* r->prev->next = r->next */ ecode = tdb_write_off(tdb, off, r->next); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - return -1; + return ecode; } if (r->next != 0) { @@ -141,27 +139,26 @@ static int remove_from_list(struct tdb_context *tdb, #ifdef CCAN_TDB2_DEBUG if (tdb_read_off(tdb, off) & TDB_OFF_MASK != r_off) { - tdb_logerr(tdb, TDB_ERR_CORRUPT, TDB_LOG_ERROR, - "remove_from_list: %llu bad list %llu", - (long long)r_off, (long long)b_off); - return -1; + return tdb_logerr(tdb, TDB_ERR_CORRUPT, TDB_LOG_ERROR, + "remove_from_list:" + " %llu bad list %llu", + (long long)r_off, (long long)b_off); } #endif ecode = tdb_write_off(tdb, off, r->magic_and_prev); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - return -1; + return ecode; } } - return 0; + return TDB_SUCCESS; } /* Enqueue in this free bucket. */ -static int enqueue_in_free(struct tdb_context *tdb, - tdb_off_t b_off, - tdb_off_t off, - tdb_len_t len) +static enum TDB_ERROR enqueue_in_free(struct tdb_context *tdb, + tdb_off_t b_off, + tdb_off_t off, + tdb_len_t len) { struct tdb_free_record new; enum TDB_ERROR ecode; @@ -176,8 +173,7 @@ static int enqueue_in_free(struct tdb_context *tdb, /* new->next = head. */ new.next = tdb_read_off(tdb, b_off); if (TDB_OFF_IS_ERR(new.next)) { - tdb->ecode = new.next; - return -1; + return new.next; } if (new.next) { @@ -186,11 +182,11 @@ static int enqueue_in_free(struct tdb_context *tdb, new.next + offsetof(struct tdb_free_record, magic_and_prev)) != magic) { - tdb_logerr(tdb, TDB_ERR_CORRUPT, TDB_LOG_ERROR, - "enqueue_in_free: %llu bad head" - " prev %llu", - (long long)new.next, (long long)b_off); - return -1; + return tdb_logerr(tdb, TDB_ERR_CORRUPT, TDB_LOG_ERROR, + "enqueue_in_free: %llu bad head" + " prev %llu", + (long long)new.next, + (long long)b_off); } #endif /* next->prev = new. */ @@ -199,32 +195,24 @@ static int enqueue_in_free(struct tdb_context *tdb, magic_and_prev), off | magic); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - return -1; + return ecode; } } /* head = new */ ecode = tdb_write_off(tdb, b_off, off); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - return -1; + return ecode; } - ecode = tdb_write_convert(tdb, off, &new, sizeof(new)); - if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - return -1; - } - return 0; + return tdb_write_convert(tdb, off, &new, sizeof(new)); } /* List need not be locked. */ -int add_free_record(struct tdb_context *tdb, - tdb_off_t off, tdb_len_t len_with_header) +enum TDB_ERROR add_free_record(struct tdb_context *tdb, + tdb_off_t off, tdb_len_t len_with_header) { tdb_off_t b_off; tdb_len_t len; - int ret; enum TDB_ERROR ecode; assert(len_with_header >= sizeof(struct tdb_free_record)); @@ -234,13 +222,12 @@ int add_free_record(struct tdb_context *tdb, b_off = bucket_off(tdb->ftable_off, size_to_bucket(len)); ecode = tdb_lock_free_bucket(tdb, b_off, TDB_LOCK_WAIT); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - return -1; + return ecode; } - ret = enqueue_in_free(tdb, b_off, off, len); + ecode = enqueue_in_free(tdb, b_off, off, len); tdb_unlock_free_bucket(tdb, b_off); - return ret; + return ecode; } static size_t adjust_size(size_t keylen, size_t datalen) @@ -290,8 +277,9 @@ static tdb_off_t ftable_offset(struct tdb_context *tdb, unsigned int ftable) } /* Note: we unlock the current bucket if we coalesce or fail. */ -static int coalesce(struct tdb_context *tdb, - tdb_off_t off, tdb_off_t b_off, tdb_len_t data_len) +static tdb_bool_err coalesce(struct tdb_context *tdb, + tdb_off_t off, tdb_off_t b_off, + tdb_len_t data_len) { tdb_off_t end; struct tdb_free_record rec; @@ -307,7 +295,7 @@ static int coalesce(struct tdb_context *tdb, r = tdb_access_read(tdb, end, sizeof(*r), true); if (TDB_PTR_IS_ERR(r)) { - tdb->ecode = TDB_PTR_ERR(r); + ecode = TDB_PTR_ERR(r); goto err; } @@ -319,7 +307,13 @@ static int coalesce(struct tdb_context *tdb, ftable = frec_ftable(r); bucket = size_to_bucket(frec_len(r)); - nb_off = bucket_off(ftable_offset(tdb, ftable), bucket); + nb_off = ftable_offset(tdb, ftable); + if (TDB_OFF_IS_ERR(nb_off)) { + tdb_access_release(tdb, r); + ecode = nb_off; + goto err; + } + nb_off = bucket_off(nb_off, bucket); tdb_access_release(tdb, r); /* We may be violating lock order here, so best effort. */ @@ -332,7 +326,6 @@ static int coalesce(struct tdb_context *tdb, /* Now we have lock, re-check. */ ecode = tdb_read_convert(tdb, end, &rec, sizeof(rec)); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; tdb_unlock_free_bucket(tdb, nb_off); goto err; } @@ -350,7 +343,8 @@ static int coalesce(struct tdb_context *tdb, break; } - if (remove_from_list(tdb, nb_off, end, &rec) == -1) { + ecode = remove_from_list(tdb, nb_off, end, &rec); + if (ecode != TDB_SUCCESS) { tdb_unlock_free_bucket(tdb, nb_off); goto err; } @@ -362,24 +356,25 @@ static int coalesce(struct tdb_context *tdb, /* Didn't find any adjacent free? */ if (end == off + sizeof(struct tdb_used_record) + data_len) - return 0; + return false; /* OK, expand initial record */ ecode = tdb_read_convert(tdb, off, &rec, sizeof(rec)); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; goto err; } if (frec_len(&rec) != data_len) { - tdb_logerr(tdb, TDB_ERR_CORRUPT, TDB_LOG_ERROR, - "coalesce: expected data len %zu not %zu", - (size_t)data_len, (size_t)frec_len(&rec)); + ecode = tdb_logerr(tdb, TDB_ERR_CORRUPT, TDB_LOG_ERROR, + "coalesce: expected data len %zu not %zu", + (size_t)data_len, (size_t)frec_len(&rec)); goto err; } - if (remove_from_list(tdb, b_off, off, &rec) == -1) + ecode = remove_from_list(tdb, b_off, off, &rec); + if (ecode != TDB_SUCCESS) { goto err; + } /* We have to drop this to avoid deadlocks, so make sure record * doesn't get coalesced by someone else! */ @@ -389,21 +384,23 @@ static int coalesce(struct tdb_context *tdb, ftable_and_len), rec.ftable_and_len); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; goto err; } add_stat(tdb, alloc_coalesce_succeeded, 1); tdb_unlock_free_bucket(tdb, b_off); - if (add_free_record(tdb, off, end - off) == -1) - return -1; - return 1; + ecode = add_free_record(tdb, off, end - off); + if (ecode != TDB_SUCCESS) { + tdb->ecode = ecode; + return ecode; + } + return true; err: /* To unify error paths, we *always* unlock bucket on error. */ tdb_unlock_free_bucket(tdb, b_off); - return -1; + return ecode; } /* We need size bytes to put our key and data in. */ @@ -429,8 +426,7 @@ again: /* Lock this bucket. */ ecode = tdb_lock_free_bucket(tdb, b_off, TDB_LOCK_WAIT); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - return TDB_OFF_ERR; + return ecode; } best.ftable_and_len = -1ULL; @@ -446,7 +442,7 @@ again: * as we go. */ off = tdb_read_off(tdb, b_off); if (TDB_OFF_IS_ERR(off)) { - tdb->ecode = off; + ecode = off; goto unlock_err; } @@ -454,18 +450,21 @@ again: const struct tdb_free_record *r; tdb_len_t len; tdb_off_t next; + int coal; r = tdb_access_read(tdb, off, sizeof(*r), true); if (TDB_PTR_IS_ERR(r)) { - tdb->ecode = TDB_PTR_ERR(r); + ecode = TDB_PTR_ERR(r); goto unlock_err; } if (frec_magic(r) != TDB_FREE_MAGIC) { tdb_access_release(tdb, r); - tdb_logerr(tdb, TDB_ERR_CORRUPT, TDB_LOG_ERROR, - "lock_and_alloc: %llu non-free 0x%llx", - (long long)off, (long long)r->magic_and_prev); + ecode = tdb_logerr(tdb, TDB_ERR_CORRUPT, TDB_LOG_ERROR, + "lock_and_alloc:" + " %llu non-free 0x%llx", + (long long)off, + (long long)r->magic_and_prev); goto unlock_err; } @@ -486,14 +485,15 @@ again: tdb_access_release(tdb, r); /* Since we're going slow anyway, try coalescing here. */ - switch (coalesce(tdb, off, b_off, len)) { - case -1: - /* This has already unlocked on error. */ - return -1; - case 1: + coal = coalesce(tdb, off, b_off, len); + if (coal == 1) { /* This has unlocked list, restart. */ goto again; } + if (coal < 0) { + /* This has already unlocked on error. */ + return coal; + } off = next; } @@ -503,8 +503,10 @@ again: size_t leftover; /* We're happy with this size: take it. */ - if (remove_from_list(tdb, b_off, best_off, &best) != 0) + ecode = remove_from_list(tdb, b_off, best_off, &best); + if (ecode != TDB_SUCCESS) { goto unlock_err; + } leftover = record_leftover(keylen, datalen, want_extra, frec_len(&best)); @@ -512,13 +514,14 @@ again: assert(keylen + datalen + leftover <= frec_len(&best)); /* We need to mark non-free before we drop lock, otherwise * coalesce() could try to merge it! */ - if (set_header(tdb, &rec, magic, keylen, datalen, - frec_len(&best) - leftover, hashlow) != 0) + ecode = set_header(tdb, &rec, magic, keylen, datalen, + frec_len(&best) - leftover, hashlow); + if (ecode != TDB_SUCCESS) { goto unlock_err; + } ecode = tdb_write_convert(tdb, best_off, &rec, sizeof(rec)); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; goto unlock_err; } @@ -526,11 +529,13 @@ again: * locking is allowed. */ if (leftover) { add_stat(tdb, alloc_leftover, 1); - if (add_free_record(tdb, - best_off + sizeof(rec) - + frec_len(&best) - leftover, - leftover)) - best_off = TDB_OFF_ERR; + ecode = add_free_record(tdb, + best_off + sizeof(rec) + + frec_len(&best) - leftover, + leftover); + if (ecode != TDB_SUCCESS) { + best_off = ecode; + } } tdb_unlock_free_bucket(tdb, b_off); @@ -542,10 +547,10 @@ again: unlock_err: tdb_unlock_free_bucket(tdb, b_off); - return TDB_OFF_ERR; + return ecode; } -/* Get a free block from current free list, or 0 if none. */ +/* Get a free block from current free list, or 0 if none, -ve on error. */ static tdb_off_t get_free(struct tdb_context *tdb, size_t keylen, size_t datalen, bool want_extra, unsigned magic, unsigned hashlow) @@ -572,8 +577,8 @@ static tdb_off_t get_free(struct tdb_context *tdb, off = lock_and_alloc(tdb, ftable_off, b, keylen, datalen, want_extra, magic, hashlow); - if (off == TDB_OFF_ERR) - return TDB_OFF_ERR; + if (TDB_OFF_IS_ERR(off)) + return off; if (off != 0) { if (b == start_b) add_stat(tdb, alloc_bucket_exact, 1); @@ -588,15 +593,13 @@ static tdb_off_t get_free(struct tdb_context *tdb, } if (TDB_OFF_IS_ERR(b)) { - tdb->ecode = b; - return 0; + return b; } /* Hmm, try next table. */ ftable_off = next_ftable(tdb, ftable_off); if (TDB_OFF_IS_ERR(ftable_off)) { - tdb->ecode = ftable_off; - return 0; + return ftable_off; } ftable++; @@ -604,8 +607,7 @@ static tdb_off_t get_free(struct tdb_context *tdb, wrapped = true; ftable_off = first_ftable(tdb); if (TDB_OFF_IS_ERR(ftable_off)) { - tdb->ecode = ftable_off; - return 0; + return ftable_off; } ftable = 0; } @@ -614,10 +616,10 @@ static tdb_off_t get_free(struct tdb_context *tdb, return 0; } -int set_header(struct tdb_context *tdb, - struct tdb_used_record *rec, - unsigned magic, uint64_t keylen, uint64_t datalen, - uint64_t actuallen, unsigned hashlow) +enum TDB_ERROR set_header(struct tdb_context *tdb, + struct tdb_used_record *rec, + unsigned magic, uint64_t keylen, uint64_t datalen, + uint64_t actuallen, unsigned hashlow) { uint64_t keybits = (fls64(keylen) + 1) / 2; @@ -632,17 +634,16 @@ int set_header(struct tdb_context *tdb, if (rec_key_length(rec) != keylen || rec_data_length(rec) != datalen || rec_extra_padding(rec) != actuallen - (keylen + datalen)) { - tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, - "Could not encode k=%llu,d=%llu,a=%llu", - (long long)keylen, (long long)datalen, - (long long)actuallen); - return -1; + return tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, + "Could not encode k=%llu,d=%llu,a=%llu", + (long long)keylen, (long long)datalen, + (long long)actuallen); } - return 0; + return TDB_SUCCESS; } /* Expand the database. */ -static int tdb_expand(struct tdb_context *tdb, tdb_len_t size) +static enum TDB_ERROR tdb_expand(struct tdb_context *tdb, tdb_len_t size) { uint64_t old_size; tdb_len_t wanted; @@ -654,9 +655,8 @@ static int tdb_expand(struct tdb_context *tdb, tdb_len_t size) /* Need to hold a hash lock to expand DB: transactions rely on it. */ if (!(tdb->flags & TDB_NOLOCK) && !tdb->allrecord_lock.count && !tdb_has_hash_locks(tdb)) { - tdb_logerr(tdb, TDB_ERR_LOCK, TDB_LOG_ERROR, - "tdb_expand: must hold lock during expand"); - return -1; + return tdb_logerr(tdb, TDB_ERR_LOCK, TDB_LOG_ERROR, + "tdb_expand: must hold lock during expand"); } /* always make room for at least 100 more records, and at @@ -670,8 +670,7 @@ static int tdb_expand(struct tdb_context *tdb, tdb_len_t size) /* Only one person can expand file at a time. */ ecode = tdb_lock_expand(tdb, F_WRLCK); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - return -1; + return ecode; } /* Someone else may have expanded the file, so retry. */ @@ -679,14 +678,13 @@ static int tdb_expand(struct tdb_context *tdb, tdb_len_t size) tdb->methods->oob(tdb, tdb->map_size + 1, true); if (tdb->map_size != old_size) { tdb_unlock_expand(tdb, F_WRLCK); - return 0; + return TDB_SUCCESS; } ecode = tdb->methods->expand_file(tdb, wanted); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; tdb_unlock_expand(tdb, F_WRLCK); - return -1; + return ecode; } /* We need to drop this lock before adding free record. */ @@ -706,12 +704,15 @@ tdb_off_t alloc(struct tdb_context *tdb, size_t keylen, size_t datalen, assert(!tdb->direct_access); for (;;) { + enum TDB_ERROR ecode; off = get_free(tdb, keylen, datalen, growing, magic, hash); if (likely(off != 0)) break; - if (tdb_expand(tdb, adjust_size(keylen, datalen))) - return TDB_OFF_ERR; + ecode = tdb_expand(tdb, adjust_size(keylen, datalen)); + if (ecode != TDB_SUCCESS) { + return ecode; + } } return off; diff --git a/ccan/tdb2/hash.c b/ccan/tdb2/hash.c index 6340267c..5b44678e 100644 --- a/ccan/tdb2/hash.c +++ b/ccan/tdb2/hash.c @@ -448,8 +448,8 @@ static enum TDB_ERROR COLD add_to_chain(struct tdb_context *tdb, if (!next) { next = alloc(tdb, 0, sizeof(struct tdb_chain), 0, TDB_CHAIN_MAGIC, false); - if (next == TDB_OFF_ERR) - return tdb->ecode; + if (TDB_OFF_IS_ERR(next)) + return next; ecode = zero_out(tdb, next+sizeof(struct tdb_used_record), sizeof(struct tdb_chain)); @@ -521,8 +521,8 @@ static enum TDB_ERROR expand_group(struct tdb_context *tdb, struct hash_info *h) } subhash = alloc(tdb, 0, subsize, 0, magic, false); - if (subhash == TDB_OFF_ERR) { - return tdb->ecode; + if (TDB_OFF_IS_ERR(subhash)) { + return subhash; } ecode = zero_out(tdb, subhash + sizeof(struct tdb_used_record), diff --git a/ccan/tdb2/private.h b/ccan/tdb2/private.h index 7a5809bc..236aa369 100644 --- a/ccan/tdb2/private.h +++ b/ccan/tdb2/private.h @@ -73,7 +73,6 @@ typedef uint64_t tdb_off_t; #define TDB_RECOVERY_MAGIC (0xf53bc0e7ad124589ULL) #define TDB_RECOVERY_INVALID_MAGIC (0x0ULL) -#define TDB_OFF_ERR ((tdb_off_t)-1) #define TDB_OFF_IS_ERR(off) unlikely(off >= (tdb_off_t)TDB_ERR_LAST) /* Packing errors into pointers and v.v. */ @@ -429,25 +428,25 @@ enum TDB_ERROR delete_from_hash(struct tdb_context *tdb, struct hash_info *h); bool is_subhash(tdb_off_t val); /* free.c: */ -int tdb_ftable_init(struct tdb_context *tdb); +enum TDB_ERROR tdb_ftable_init(struct tdb_context *tdb); /* check.c needs these to iterate through free lists. */ tdb_off_t first_ftable(struct tdb_context *tdb); tdb_off_t next_ftable(struct tdb_context *tdb, tdb_off_t ftable); -/* This returns space or TDB_OFF_ERR. */ +/* This returns space or -ve error number. */ tdb_off_t alloc(struct tdb_context *tdb, size_t keylen, size_t datalen, uint64_t hash, unsigned magic, bool growing); /* Put this record in a free list. */ -int add_free_record(struct tdb_context *tdb, - tdb_off_t off, tdb_len_t len_with_header); +enum TDB_ERROR add_free_record(struct tdb_context *tdb, + tdb_off_t off, tdb_len_t len_with_header); /* Set up header for a used/ftable/htable/chain record. */ -int set_header(struct tdb_context *tdb, - struct tdb_used_record *rec, - unsigned magic, uint64_t keylen, uint64_t datalen, - uint64_t actuallen, unsigned hashlow); +enum TDB_ERROR set_header(struct tdb_context *tdb, + struct tdb_used_record *rec, + unsigned magic, uint64_t keylen, uint64_t datalen, + uint64_t actuallen, unsigned hashlow); /* Used by tdb_check to verify. */ unsigned int size_to_bucket(tdb_len_t data_len); diff --git a/ccan/tdb2/tdb.c b/ccan/tdb2/tdb.c index 348e11ce..2b0b77ad 100644 --- a/ccan/tdb2/tdb.c +++ b/ccan/tdb2/tdb.c @@ -117,9 +117,13 @@ static int tdb_new_database(struct tdb_context *tdb, /* Free is empty. */ newdb.hdr.free_table = offsetof(struct new_database, ftable); memset(&newdb.ftable, 0, sizeof(newdb.ftable)); - set_header(NULL, &newdb.ftable.hdr, TDB_FTABLE_MAGIC, 0, - sizeof(newdb.ftable) - sizeof(newdb.ftable.hdr), - sizeof(newdb.ftable) - sizeof(newdb.ftable.hdr), 0); + tdb->ecode = set_header(NULL, &newdb.ftable.hdr, TDB_FTABLE_MAGIC, 0, + sizeof(newdb.ftable) - sizeof(newdb.ftable.hdr), + sizeof(newdb.ftable) - sizeof(newdb.ftable.hdr), + 0); + if (tdb->ecode != TDB_SUCCESS) { + return -1; + } /* Magic food */ memset(newdb.hdr.magic_food, 0, sizeof(newdb.hdr.magic_food)); @@ -354,8 +358,10 @@ struct tdb_context *tdb_open(const char *name, int tdb_flags, } } - if (tdb_ftable_init(tdb) == -1) + tdb->ecode = tdb_ftable_init(tdb); + if (tdb->ecode != TDB_SUCCESS) { goto fail; + } tdb->next = tdbs; tdbs = tdb; @@ -415,9 +421,12 @@ static int update_rec_hdr(struct tdb_context *tdb, uint64_t dataroom = rec_data_length(rec) + rec_extra_padding(rec); enum TDB_ERROR ecode; - if (set_header(tdb, rec, TDB_USED_MAGIC, keylen, datalen, - keylen + dataroom, h)) + ecode = set_header(tdb, rec, TDB_USED_MAGIC, keylen, datalen, + keylen + dataroom, h); + if (ecode != TDB_SUCCESS) { + tdb->ecode = ecode; return -1; + } ecode = tdb_write_convert(tdb, off, rec, sizeof(*rec)); if (ecode != TDB_SUCCESS) { @@ -440,16 +449,19 @@ static int replace_data(struct tdb_context *tdb, /* Allocate a new record. */ new_off = alloc(tdb, key.dsize, dbuf.dsize, h->h, TDB_USED_MAGIC, growing); - if (unlikely(new_off == TDB_OFF_ERR)) + if (TDB_OFF_IS_ERR(new_off)) { + tdb->ecode = new_off; return -1; + } /* We didn't like the existing one: remove it. */ if (old_off) { add_stat(tdb, frees, 1); - add_free_record(tdb, old_off, - sizeof(struct tdb_used_record) - + key.dsize + old_room); - ecode = replace_in_hash(tdb, h, new_off); + ecode = add_free_record(tdb, old_off, + sizeof(struct tdb_used_record) + + key.dsize + old_room); + if (ecode == TDB_SUCCESS) + ecode = replace_in_hash(tdb, h, new_off); } else { ecode = add_to_hash(tdb, h, new_off); } @@ -676,12 +688,15 @@ int tdb_delete(struct tdb_context *tdb, struct tdb_data key) /* Free the deleted entry. */ add_stat(tdb, frees, 1); - if (add_free_record(tdb, off, - sizeof(struct tdb_used_record) - + rec_key_length(&rec) - + rec_data_length(&rec) - + rec_extra_padding(&rec)) != 0) + ecode = add_free_record(tdb, off, + sizeof(struct tdb_used_record) + + rec_key_length(&rec) + + rec_data_length(&rec) + + rec_extra_padding(&rec)); + if (ecode != TDB_SUCCESS) { + tdb->ecode = ecode; goto unlock_err; + } tdb_unlock_hashes(tdb, h.hlock_start, h.hlock_range, F_WRLCK); return 0; diff --git a/ccan/tdb2/test/run-01-new_database.c b/ccan/tdb2/test/run-01-new_database.c index 49f2580b..ee6cd55d 100644 --- a/ccan/tdb2/test/run-01-new_database.c +++ b/ccan/tdb2/test/run-01-new_database.c @@ -11,8 +11,8 @@ #include "logging.h" /* FIXME: Check these! */ -#define INITIAL_TDB_MALLOC "tdb.c", 178, FAILTEST_MALLOC -#define LOGGING_MALLOC "tdb.c", 734, FAILTEST_MALLOC +#define INITIAL_TDB_MALLOC "tdb.c", 182, FAILTEST_MALLOC +#define LOGGING_MALLOC "tdb.c", 792, FAILTEST_MALLOC #define URANDOM_OPEN "tdb.c", 49, FAILTEST_OPEN #define URANDOM_READ "tdb.c", 29, FAILTEST_READ diff --git a/ccan/tdb2/test/run-04-basichash.c b/ccan/tdb2/test/run-04-basichash.c index d1e41c29..0745a23c 100644 --- a/ccan/tdb2/test/run-04-basichash.c +++ b/ccan/tdb2/test/run-04-basichash.c @@ -67,7 +67,7 @@ int main(int argc, char *argv[]) /* Allocate a new record. */ new_off = alloc(tdb, key.dsize, dbuf.dsize, h.h, TDB_USED_MAGIC, false); - ok1(new_off != TDB_OFF_ERR); + ok1(!TDB_OFF_IS_ERR(new_off)); /* We should be able to add it now. */ ok1(add_to_hash(tdb, &h, new_off) == 0); @@ -228,7 +228,7 @@ int main(int argc, char *argv[]) /* Allocate a new record. */ new_off = alloc(tdb, key.dsize, dbuf.dsize, h.h, TDB_USED_MAGIC, false); - ok1(new_off != TDB_OFF_ERR); + ok1(!TDB_OFF_IS_ERR(new_off)); ok1(add_to_hash(tdb, &h, new_off) == 0); /* Make sure we fill it in for later finding. */ diff --git a/ccan/tdb2/transaction.c b/ccan/tdb2/transaction.c index d05b1fef..604861b7 100644 --- a/ccan/tdb2/transaction.c +++ b/ccan/tdb2/transaction.c @@ -681,9 +681,10 @@ static int tdb_recovery_allocate(struct tdb_context *tdb, the transaction) */ if (recovery_head != 0) { add_stat(tdb, frees, 1); - if (add_free_record(tdb, recovery_head, - sizeof(rec) + rec.max_len) != 0) { - tdb_logerr(tdb, tdb->ecode, TDB_LOG_ERROR, + ecode = add_free_record(tdb, recovery_head, + sizeof(rec) + rec.max_len); + if (ecode != TDB_SUCCESS) { + tdb_logerr(tdb, ecode, TDB_LOG_ERROR, "tdb_recovery_allocate:" " failed to free previous recovery area"); return -1; -- 2.39.2