From b21004624683be5bf1d8f75e3b5be4e9618049ee Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 1 Mar 2011 23:19:19 +1030 Subject: [PATCH] tdb2: change API to return the error value. Mostly a fairly simple transformation, since 0 still means success. One new twist is that tdb_nextkey now frees the .dptr of the key; this us usually what we want but does cause issues for our weird test code. --- ccan/tdb2/_info | 14 +- ccan/tdb2/check.c | 39 +-- ccan/tdb2/free.c | 2 - ccan/tdb2/hash.c | 21 +- ccan/tdb2/summary.c | 105 ++++----- ccan/tdb2/tdb.c | 223 ++++++++---------- ccan/tdb2/tdb2.h | 130 +++++----- ccan/tdb2/test/external-agent.c | 15 +- ccan/tdb2/test/run-01-new_database.c | 4 +- ccan/tdb2/test/run-10-simple-store.c | 10 +- ccan/tdb2/test/run-11-simple-fetch.c | 6 +- ccan/tdb2/test/run-12-store.c | 4 +- ccan/tdb2/test/run-13-delete.c | 23 +- ccan/tdb2/test/run-15-append.c | 8 +- ccan/tdb2/test/run-25-hashoverload.c | 8 +- ccan/tdb2/test/run-55-transaction.c | 12 +- .../tdb2/test/run-57-die-during-transaction.c | 4 + ccan/tdb2/test/run-firstkey-nextkey.c | 55 +++-- ccan/tdb2/test/run-simple-delete.c | 5 +- ccan/tdb2/test/run-summary.c | 4 +- ccan/tdb2/test/run-tdb_errorstr.c | 90 ++++--- ccan/tdb2/test/run-traverse.c | 8 +- ccan/tdb2/transaction.c | 67 +++--- ccan/tdb2/traverse.c | 36 +-- 24 files changed, 395 insertions(+), 498 deletions(-) diff --git a/ccan/tdb2/_info b/ccan/tdb2/_info index ae740cc7..114b60b2 100644 --- a/ccan/tdb2/_info +++ b/ccan/tdb2/_info @@ -24,7 +24,8 @@ * { * struct tdb_context *tdb; * TDB_DATA key, value; - * + * enum TDB_ERROR error; + * * if (argc < 4) * usage(argv[0]); * @@ -38,10 +39,10 @@ * if (streq(argv[1], "fetch")) { * if (argc != 4) * usage(argv[0]); - * value = tdb_fetch(tdb, key); - * if (!value.dptr) + * error = tdb_fetch(tdb, key, &value); + * if (error) * errx(1, "fetch %s: %s", - * argv[3], tdb_errorstr(tdb)); + * argv[3], tdb_errorstr(error)); * printf("%.*s\n", value.dsize, (char *)value.dptr); * free(value.dptr); * } else if (streq(argv[1], "store")) { @@ -49,9 +50,10 @@ * usage(argv[0]); * value.dptr = (void *)argv[4]; * value.dsize = strlen(argv[4]); - * if (tdb_store(tdb, key, value, 0) != 0) + * error = tdb_store(tdb, key, value, 0); + * if (error) * errx(1, "store %s: %s", - * argv[3], tdb_errorstr(tdb)); + * argv[3], tdb_errorstr(error)); * } else * usage(argv[0]); * diff --git a/ccan/tdb2/check.c b/ccan/tdb2/check.c index 4f340a2e..de1cb98b 100644 --- a/ccan/tdb2/check.c +++ b/ccan/tdb2/check.c @@ -80,7 +80,8 @@ static enum TDB_ERROR check_hash_tree(struct tdb_context *tdb, tdb_off_t used[], size_t num_used, size_t *num_found, - int (*check)(TDB_DATA, TDB_DATA, void *), + enum TDB_ERROR (*check)(TDB_DATA, + TDB_DATA, void *), void *private_data); static enum TDB_ERROR check_hash_chain(struct tdb_context *tdb, @@ -89,7 +90,9 @@ static enum TDB_ERROR check_hash_chain(struct tdb_context *tdb, tdb_off_t used[], size_t num_used, size_t *num_found, - int (*check)(TDB_DATA, TDB_DATA, void *), + enum TDB_ERROR (*check)(TDB_DATA, + TDB_DATA, + void *), void *private_data) { struct tdb_used_record rec; @@ -149,7 +152,9 @@ static enum TDB_ERROR check_hash_record(struct tdb_context *tdb, tdb_off_t used[], size_t num_used, size_t *num_found, - int (*check)(TDB_DATA, TDB_DATA, void*), + enum TDB_ERROR (*check)(TDB_DATA, + TDB_DATA, + void *), void *private_data) { struct tdb_used_record rec; @@ -218,7 +223,8 @@ static enum TDB_ERROR check_hash_tree(struct tdb_context *tdb, tdb_off_t used[], size_t num_used, size_t *num_found, - int (*check)(TDB_DATA, TDB_DATA, void *), + enum TDB_ERROR (*check)(TDB_DATA, + TDB_DATA, void *), void *private_data) { unsigned int g, b; @@ -395,8 +401,8 @@ static enum TDB_ERROR check_hash_tree(struct tdb_context *tdb, goto fail; } data.dptr = key.dptr + key.dsize; - if (check(key, data, private_data) != 0) { - ecode = TDB_ERR_CORRUPT; + ecode = check(key, data, private_data); + if (ecode != TDB_SUCCESS) { goto fail; } tdb_access_release(tdb, key.dptr); @@ -711,9 +717,10 @@ static enum TDB_ERROR check_linear(struct tdb_context *tdb, return TDB_SUCCESS; } -int tdb_check(struct tdb_context *tdb, - int (*check)(TDB_DATA key, TDB_DATA data, void *private_data), - void *private_data) +enum TDB_ERROR tdb_check(struct tdb_context *tdb, + enum TDB_ERROR (*check)(TDB_DATA key, TDB_DATA data, + void *private_data), + void *private_data) { tdb_off_t *fr = NULL, *used = NULL, ft, recovery; size_t num_free = 0, num_used = 0, num_found = 0, num_ftables = 0; @@ -721,15 +728,13 @@ int tdb_check(struct tdb_context *tdb, ecode = tdb_allrecord_lock(tdb, F_RDLCK, TDB_LOCK_WAIT, false); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - return -1; + return ecode; } ecode = tdb_lock_expand(tdb, F_RDLCK); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; tdb_allrecord_unlock(tdb, F_RDLCK); - return -1; + return ecode; } ecode = check_header(tdb, &recovery); @@ -743,7 +748,7 @@ int tdb_check(struct tdb_context *tdb, for (ft = first_ftable(tdb); ft; ft = next_ftable(tdb, ft)) { if (TDB_OFF_IS_ERR(ft)) { - tdb->ecode = ft; + ecode = ft; goto out; } ecode = check_free_table(tdb, ft, num_ftables, fr, num_free, @@ -770,9 +775,5 @@ out: tdb_unlock_expand(tdb, F_RDLCK); free(fr); free(used); - if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - return -1; - } - return 0; + return ecode; } diff --git a/ccan/tdb2/free.c b/ccan/tdb2/free.c index 97320b58..d2285567 100644 --- a/ccan/tdb2/free.c +++ b/ccan/tdb2/free.c @@ -268,7 +268,6 @@ static tdb_off_t ftable_offset(struct tdb_context *tdb, unsigned int ftable) off = first_ftable(tdb); for (i = 0; i < ftable; i++) { if (TDB_OFF_IS_ERR(off)) { - tdb->ecode = off; break; } off = next_ftable(tdb, off); @@ -392,7 +391,6 @@ static tdb_bool_err coalesce(struct tdb_context *tdb, ecode = add_free_record(tdb, off, end - off); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; return ecode; } return true; diff --git a/ccan/tdb2/hash.c b/ccan/tdb2/hash.c index 5b44678e..9ebc1fe2 100644 --- a/ccan/tdb2/hash.c +++ b/ccan/tdb2/hash.c @@ -48,7 +48,6 @@ uint64_t hash_record(struct tdb_context *tdb, tdb_off_t off) r = tdb_access_read(tdb, off, sizeof(*r), true); if (TDB_PTR_IS_ERR(r)) { - tdb->ecode = TDB_PTR_ERR(r); /* FIXME */ return 0; } @@ -58,7 +57,6 @@ uint64_t hash_record(struct tdb_context *tdb, tdb_off_t off) key = tdb_access_read(tdb, off + sizeof(*r), klen, false); if (TDB_PTR_IS_ERR(key)) { - tdb->ecode = TDB_PTR_ERR(key); return 0; } @@ -857,22 +855,16 @@ static enum TDB_ERROR chainlock(struct tdb_context *tdb, const TDB_DATA *key, /* lock/unlock one hash chain. This is meant to be used to reduce contention - it cannot guarantee how many records will be locked */ -int tdb_chainlock(struct tdb_context *tdb, TDB_DATA key) +enum TDB_ERROR tdb_chainlock(struct tdb_context *tdb, TDB_DATA key) { - tdb->ecode = chainlock(tdb, &key, F_WRLCK, TDB_LOCK_WAIT, - "tdb_chainlock"); - if (tdb->ecode == TDB_SUCCESS) - return 0; - return -1; - + return chainlock(tdb, &key, F_WRLCK, TDB_LOCK_WAIT, "tdb_chainlock"); } -int tdb_chainunlock(struct tdb_context *tdb, TDB_DATA key) +enum TDB_ERROR tdb_chainunlock(struct tdb_context *tdb, TDB_DATA key) { uint64_t h = tdb_hash(tdb, key.dptr, key.dsize); tdb_off_t lockstart, locksize; unsigned int group, gbits; - enum TDB_ERROR ecode; gbits = TDB_TOPLEVEL_HASH_BITS - TDB_HASH_GROUP_BITS; group = bits_from(h, 64 - gbits, gbits); @@ -880,10 +872,5 @@ int tdb_chainunlock(struct tdb_context *tdb, TDB_DATA key) lockstart = hlock_range(group, &locksize); tdb_trace_1rec(tdb, "tdb_chainunlock", key); - ecode = tdb_unlock_hashes(tdb, lockstart, locksize, F_WRLCK); - if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - return -1; - } - return 0; + return tdb_unlock_hashes(tdb, lockstart, locksize, F_WRLCK); } diff --git a/ccan/tdb2/summary.c b/ccan/tdb2/summary.c index 8cdb71b0..c26e3c5e 100644 --- a/ccan/tdb2/summary.c +++ b/ccan/tdb2/summary.c @@ -153,28 +153,27 @@ static enum TDB_ERROR summarize(struct tdb_context *tdb, #define HISTO_WIDTH 70 #define HISTO_HEIGHT 20 -char *tdb_summary(struct tdb_context *tdb, enum tdb_summary_flags flags) +enum TDB_ERROR tdb_summary(struct tdb_context *tdb, + enum tdb_summary_flags flags, + char **summary) { tdb_len_t len; struct tally *ftables, *hashes, *freet, *keys, *data, *extra, *uncoal, *buckets, *chains; char *hashesg, *freeg, *keysg, *datag, *extrag, *uncoalg, *bucketsg; - char *ret = NULL; enum TDB_ERROR ecode; hashesg = freeg = keysg = datag = extrag = uncoalg = bucketsg = NULL; ecode = tdb_allrecord_lock(tdb, F_RDLCK, TDB_LOCK_WAIT, false); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - return NULL; + return ecode; } ecode = tdb_lock_expand(tdb, F_RDLCK); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; tdb_allrecord_unlock(tdb, F_RDLCK); - return NULL; + return ecode; } /* Start stats off empty. */ @@ -189,15 +188,15 @@ char *tdb_summary(struct tdb_context *tdb, enum tdb_summary_flags flags) chains = tally_new(HISTO_HEIGHT); if (!ftables || !hashes || !freet || !keys || !data || !extra || !uncoal || !buckets || !chains) { - tdb_logerr(tdb, TDB_ERR_OOM, TDB_LOG_ERROR, - "tdb_summary: failed to allocate tally structures"); + ecode = tdb_logerr(tdb, TDB_ERR_OOM, TDB_LOG_ERROR, + "tdb_summary: failed to allocate" + " tally structures"); goto unlock; } ecode = summarize(tdb, hashes, ftables, freet, keys, data, extra, uncoal, buckets, chains); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; goto unlock; } @@ -221,52 +220,52 @@ char *tdb_summary(struct tdb_context *tdb, enum tdb_summary_flags flags) + (uncoalg ? strlen(uncoalg) : 0) + (bucketsg ? strlen(bucketsg) : 0); - ret = malloc(len); - if (!ret) { - tdb_logerr(tdb, TDB_ERR_OOM, TDB_LOG_ERROR, - "tdb_summary: failed to allocate string"); + *summary = malloc(len); + if (!*summary) { + ecode = tdb_logerr(tdb, TDB_ERR_OOM, TDB_LOG_ERROR, + "tdb_summary: failed to allocate string"); goto unlock; } - len = sprintf(ret, SUMMARY_FORMAT, - (size_t)tdb->map_size, - tally_num(keys) + tally_num(data), - tally_num(keys), - tally_min(keys), tally_mean(keys), tally_max(keys), - keysg ? keysg : "", - tally_min(data), tally_mean(data), tally_max(data), - datag ? datag : "", - tally_min(extra), tally_mean(extra), tally_max(extra), - extrag ? extrag : "", - tally_num(freet), - tally_min(freet), tally_mean(freet), tally_max(freet), - freeg ? freeg : "", - tally_total(uncoal, NULL), - tally_min(uncoal), tally_mean(uncoal), tally_max(uncoal), - uncoalg ? uncoalg : "", - tally_num(buckets), - bucketsg ? bucketsg : "", - (unsigned)count_hash(tdb, offsetof(struct tdb_header, - hashtable), - TDB_TOPLEVEL_HASH_BITS), - 1 << TDB_TOPLEVEL_HASH_BITS, - tally_num(chains), - tally_num(hashes), - tally_min(hashes), tally_mean(hashes), tally_max(hashes), - hashesg ? hashesg : "", - tally_total(keys, NULL) * 100.0 / tdb->map_size, - tally_total(data, NULL) * 100.0 / tdb->map_size, - tally_total(extra, NULL) * 100.0 / tdb->map_size, - tally_total(freet, NULL) * 100.0 / tdb->map_size, - (tally_num(keys) + tally_num(freet) + tally_num(hashes)) - * sizeof(struct tdb_used_record) * 100.0 / tdb->map_size, - tally_num(ftables) * sizeof(struct tdb_freetable) - * 100.0 / tdb->map_size, - (tally_num(hashes) - * (sizeof(tdb_off_t) << TDB_SUBLEVEL_HASH_BITS) - + (sizeof(tdb_off_t) << TDB_TOPLEVEL_HASH_BITS) - + sizeof(struct tdb_chain) * tally_num(chains)) - * 100.0 / tdb->map_size); + sprintf(*summary, SUMMARY_FORMAT, + (size_t)tdb->map_size, + tally_num(keys) + tally_num(data), + tally_num(keys), + tally_min(keys), tally_mean(keys), tally_max(keys), + keysg ? keysg : "", + tally_min(data), tally_mean(data), tally_max(data), + datag ? datag : "", + tally_min(extra), tally_mean(extra), tally_max(extra), + extrag ? extrag : "", + tally_num(freet), + tally_min(freet), tally_mean(freet), tally_max(freet), + freeg ? freeg : "", + tally_total(uncoal, NULL), + tally_min(uncoal), tally_mean(uncoal), tally_max(uncoal), + uncoalg ? uncoalg : "", + tally_num(buckets), + bucketsg ? bucketsg : "", + (unsigned)count_hash(tdb, offsetof(struct tdb_header, + hashtable), + TDB_TOPLEVEL_HASH_BITS), + 1 << TDB_TOPLEVEL_HASH_BITS, + tally_num(chains), + tally_num(hashes), + tally_min(hashes), tally_mean(hashes), tally_max(hashes), + hashesg ? hashesg : "", + tally_total(keys, NULL) * 100.0 / tdb->map_size, + tally_total(data, NULL) * 100.0 / tdb->map_size, + tally_total(extra, NULL) * 100.0 / tdb->map_size, + tally_total(freet, NULL) * 100.0 / tdb->map_size, + (tally_num(keys) + tally_num(freet) + tally_num(hashes)) + * sizeof(struct tdb_used_record) * 100.0 / tdb->map_size, + tally_num(ftables) * sizeof(struct tdb_freetable) + * 100.0 / tdb->map_size, + (tally_num(hashes) + * (sizeof(tdb_off_t) << TDB_SUBLEVEL_HASH_BITS) + + (sizeof(tdb_off_t) << TDB_TOPLEVEL_HASH_BITS) + + sizeof(struct tdb_chain) * tally_num(chains)) + * 100.0 / tdb->map_size); unlock: free(hashesg); @@ -288,5 +287,5 @@ unlock: tdb_allrecord_unlock(tdb, F_RDLCK); tdb_unlock_expand(tdb, F_RDLCK); - return ret; + return ecode; } diff --git a/ccan/tdb2/tdb.c b/ccan/tdb2/tdb.c index 8bebe2af..c087aad2 100644 --- a/ccan/tdb2/tdb.c +++ b/ccan/tdb2/tdb.c @@ -197,7 +197,6 @@ struct tdb_context *tdb_open(const char *name, int tdb_flags, tdb->direct_access = 0; tdb->fd = -1; tdb->map_size = sizeof(struct tdb_header); - tdb->ecode = TDB_SUCCESS; tdb->flags = tdb_flags; tdb->logfn = NULL; tdb->transaction = NULL; @@ -227,17 +226,20 @@ struct tdb_context *tdb_open(const char *name, int tdb_flags, tdb->stats->size = sizeof(attr->stats); break; default: - tdb_logerr(tdb, TDB_ERR_EINVAL, TDB_LOG_USE_ERROR, - "tdb_open: unknown attribute type %u", - attr->base.attr); + ecode = tdb_logerr(tdb, TDB_ERR_EINVAL, + TDB_LOG_USE_ERROR, + "tdb_open:" + " unknown attribute type %u", + attr->base.attr); goto fail; } attr = attr->base.next; } if ((open_flags & O_ACCMODE) == O_WRONLY) { - tdb_logerr(tdb, TDB_ERR_EINVAL, TDB_LOG_USE_ERROR, - "tdb_open: can't open tdb %s write-only", name); + ecode = tdb_logerr(tdb, TDB_ERR_EINVAL, TDB_LOG_USE_ERROR, + "tdb_open: can't open tdb %s write-only", + name); goto fail; } @@ -254,7 +256,8 @@ struct tdb_context *tdb_open(const char *name, int tdb_flags, /* internal databases don't need any of the rest. */ if (tdb->flags & TDB_INTERNAL) { tdb->flags |= (TDB_NOLOCK | TDB_NOMMAP); - if (tdb_new_database(tdb, seed, &hdr) != 0) { + ecode = tdb_new_database(tdb, seed, &hdr); + if (ecode != TDB_SUCCESS) { goto fail; } tdb_convert(tdb, &hdr.hash_seed, sizeof(hdr.hash_seed)); @@ -266,9 +269,9 @@ struct tdb_context *tdb_open(const char *name, int tdb_flags, if ((tdb->fd = open(name, open_flags, mode)) == -1) { /* errno set by open(2) */ saved_errno = errno; - tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, - "tdb_open: could not open file %s: %s", - name, strerror(errno)); + ecode = tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, + "tdb_open: could not open file %s: %s", + name, strerror(errno)); goto fail; } @@ -277,26 +280,27 @@ struct tdb_context *tdb_open(const char *name, int tdb_flags, fcntl(tdb->fd, F_SETFD, v | FD_CLOEXEC); /* ensure there is only one process initialising at once */ - tdb->ecode = tdb_lock_open(tdb, TDB_LOCK_WAIT|TDB_LOCK_NOCHECK); - if (tdb->ecode != TDB_SUCCESS) { + ecode = tdb_lock_open(tdb, TDB_LOCK_WAIT|TDB_LOCK_NOCHECK); + if (ecode != TDB_SUCCESS) { goto fail; } /* If they used O_TRUNC, read will return 0. */ rlen = read(tdb->fd, &hdr, sizeof(hdr)); if (rlen == 0 && (open_flags & O_CREAT)) { - if (tdb_new_database(tdb, seed, &hdr) == -1) { + ecode = tdb_new_database(tdb, seed, &hdr); + if (ecode != TDB_SUCCESS) { goto fail; } } else if (rlen < 0) { - tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, - "tdb_open: error %s reading %s", - strerror(errno), name); + ecode = tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, + "tdb_open: error %s reading %s", + strerror(errno), name); goto fail; } else if (rlen < sizeof(hdr) || strcmp(hdr.magic_food, TDB_MAGIC_FOOD) != 0) { - tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, - "tdb_open: %s is not a tdb file", name); + ecode = tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, + "tdb_open: %s is not a tdb file", name); goto fail; } @@ -305,9 +309,10 @@ struct tdb_context *tdb_open(const char *name, int tdb_flags, tdb->flags |= TDB_CONVERT; else { /* wrong version */ - tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, - "tdb_open: %s is unknown version 0x%llx", - name, (long long)hdr.version); + ecode = tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, + "tdb_open:" + " %s is unknown version 0x%llx", + name, (long long)hdr.version); goto fail; } } @@ -318,34 +323,35 @@ struct tdb_context *tdb_open(const char *name, int tdb_flags, hash_test = tdb_hash(tdb, &hash_test, sizeof(hash_test)); if (hdr.hash_test != hash_test) { /* wrong hash variant */ - tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, - "tdb_open: %s uses a different hash function", - name); + ecode = tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, + "tdb_open:" + " %s uses a different hash function", + name); goto fail; } if (fstat(tdb->fd, &st) == -1) { saved_errno = errno; - tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, - "tdb_open: could not stat open %s: %s", - name, strerror(errno)); + ecode = tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR, + "tdb_open: could not stat open %s: %s", + name, strerror(errno)); goto fail; } /* Is it already in the open list? If so, fail. */ if (tdb_already_open(st.st_dev, st.st_ino)) { /* FIXME */ - tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_USE_ERROR, - "tdb_open: %s (%d,%d) is already open in this" - " process", - name, (int)st.st_dev, (int)st.st_ino); + ecode = tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_USE_ERROR, + "tdb_open: %s (%d,%d) is already open" + " in this process", + name, (int)st.st_dev, (int)st.st_ino); goto fail; } tdb->name = strdup(name); if (!tdb->name) { - tdb_logerr(tdb, TDB_ERR_OOM, TDB_LOG_ERROR, - "tdb_open: failed to allocate name"); + ecode = tdb_logerr(tdb, TDB_ERR_OOM, TDB_LOG_ERROR, + "tdb_open: failed to allocate name"); goto fail; } @@ -365,13 +371,12 @@ struct tdb_context *tdb_open(const char *name, int tdb_flags, } ecode = tdb_lock_and_recover(tdb); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; goto fail; } } - tdb->ecode = tdb_ftable_init(tdb); - if (tdb->ecode != TDB_SUCCESS) { + ecode = tdb_ftable_init(tdb); + if (ecode != TDB_SUCCESS) { goto fail; } @@ -382,7 +387,7 @@ struct tdb_context *tdb_open(const char *name, int tdb_flags, fail: /* Map ecode to some logical errno. */ if (!saved_errno) { - switch (tdb->ecode) { + switch (ecode) { case TDB_ERR_CORRUPT: case TDB_ERR_IO: saved_errno = EIO; @@ -488,8 +493,8 @@ static enum TDB_ERROR replace_data(struct tdb_context *tdb, return TDB_SUCCESS; } -int tdb_store(struct tdb_context *tdb, - struct tdb_data key, struct tdb_data dbuf, int flag) +enum TDB_ERROR tdb_store(struct tdb_context *tdb, + struct tdb_data key, struct tdb_data dbuf, int flag) { struct hash_info h; tdb_off_t off; @@ -499,15 +504,14 @@ int tdb_store(struct tdb_context *tdb, off = find_and_lock(tdb, key, F_WRLCK, &h, &rec, NULL); if (TDB_OFF_IS_ERR(off)) { - tdb->ecode = off; - return -1; + return off; } /* Now we have lock on this hash bucket. */ if (flag == TDB_INSERT) { if (off) { - tdb->ecode = TDB_ERR_EXISTS; - goto fail; + ecode = TDB_ERR_EXISTS; + goto out; } } else { if (off) { @@ -519,8 +523,7 @@ int tdb_store(struct tdb_context *tdb, key.dsize, dbuf.dsize, &rec, h.h); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - goto fail; + goto out; } ecode = tdb->methods->twrite(tdb, off + sizeof(rec) @@ -528,40 +531,32 @@ int tdb_store(struct tdb_context *tdb, dbuf.dptr, dbuf.dsize); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - goto fail; + goto out; } tdb_unlock_hashes(tdb, h.hlock_start, h.hlock_range, F_WRLCK); - return 0; + return TDB_SUCCESS; } } else { if (flag == TDB_MODIFY) { /* if the record doesn't exist and we are in TDB_MODIFY mode then we should fail the store */ - tdb->ecode = TDB_ERR_NOEXIST; - goto fail; + ecode = TDB_ERR_NOEXIST; + goto out; } } } /* If we didn't use the old record, this implies we're growing. */ ecode = replace_data(tdb, &h, key, dbuf, off, old_room, off); +out: tdb_unlock_hashes(tdb, h.hlock_start, h.hlock_range, F_WRLCK); - if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - return -1; - } - return 0; - -fail: - tdb_unlock_hashes(tdb, h.hlock_start, h.hlock_range, F_WRLCK); - return -1; + return ecode; } -int tdb_append(struct tdb_context *tdb, - struct tdb_data key, struct tdb_data dbuf) +enum TDB_ERROR tdb_append(struct tdb_context *tdb, + struct tdb_data key, struct tdb_data dbuf) { struct hash_info h; tdb_off_t off; @@ -573,8 +568,7 @@ int tdb_append(struct tdb_context *tdb, off = find_and_lock(tdb, key, F_WRLCK, &h, &rec, NULL); if (TDB_OFF_IS_ERR(off)) { - tdb->ecode = off; - return -1; + return off; } if (off) { @@ -587,38 +581,29 @@ int tdb_append(struct tdb_context *tdb, old_dlen + dbuf.dsize, &rec, h.h); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - goto fail; + goto out; } off += sizeof(rec) + key.dsize + old_dlen; ecode = tdb->methods->twrite(tdb, off, dbuf.dptr, dbuf.dsize); - if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - goto fail; - } - - /* FIXME: tdb_increment_seqnum(tdb); */ - tdb_unlock_hashes(tdb, h.hlock_start, h.hlock_range, - F_WRLCK); - return 0; + goto out; } /* Slow path. */ newdata = malloc(key.dsize + old_dlen + dbuf.dsize); if (!newdata) { - tdb_logerr(tdb, TDB_ERR_OOM, TDB_LOG_ERROR, - "tdb_append: failed to allocate %zu bytes", - (size_t)(key.dsize+old_dlen+dbuf.dsize)); - goto fail; + ecode = tdb_logerr(tdb, TDB_ERR_OOM, TDB_LOG_ERROR, + "tdb_append:" + " failed to allocate %zu bytes", + (size_t)(key.dsize + old_dlen + + dbuf.dsize)); + goto out; } ecode = tdb->methods->tread(tdb, off + sizeof(rec) + key.dsize, newdata, old_dlen); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - free(newdata); - goto fail; + goto out_free_newdata; } memcpy(newdata + old_dlen, dbuf.dptr, dbuf.dsize); new_dbuf.dptr = newdata; @@ -630,51 +615,44 @@ int tdb_append(struct tdb_context *tdb, /* If they're using tdb_append(), it implies they're growing record. */ ecode = replace_data(tdb, &h, key, new_dbuf, off, old_room, true); - tdb_unlock_hashes(tdb, h.hlock_start, h.hlock_range, F_WRLCK); - free(newdata); - if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - return -1; - } - return 0; - -fail: +out_free_newdata: + free(newdata); +out: tdb_unlock_hashes(tdb, h.hlock_start, h.hlock_range, F_WRLCK); - return -1; + return ecode; } -struct tdb_data tdb_fetch(struct tdb_context *tdb, struct tdb_data key) +enum TDB_ERROR tdb_fetch(struct tdb_context *tdb, struct tdb_data key, + struct tdb_data *data) { tdb_off_t off; struct tdb_used_record rec; struct hash_info h; - struct tdb_data ret; + enum TDB_ERROR ecode; off = find_and_lock(tdb, key, F_RDLCK, &h, &rec, NULL); if (TDB_OFF_IS_ERR(off)) { - tdb->ecode = off; - return tdb_null; + return off; } if (!off) { - tdb->ecode = TDB_ERR_NOEXIST; - ret = tdb_null; + ecode = TDB_ERR_NOEXIST; } else { - ret.dsize = rec_data_length(&rec); - ret.dptr = tdb_alloc_read(tdb, off + sizeof(rec) + key.dsize, - ret.dsize); - if (TDB_PTR_IS_ERR(ret.dptr)) { - tdb->ecode = TDB_PTR_ERR(ret.dptr); - ret = tdb_null; - } + data->dsize = rec_data_length(&rec); + data->dptr = tdb_alloc_read(tdb, off + sizeof(rec) + key.dsize, + data->dsize); + if (TDB_PTR_IS_ERR(data->dptr)) { + ecode = TDB_PTR_ERR(data->dptr); + } else + ecode = TDB_SUCCESS; } tdb_unlock_hashes(tdb, h.hlock_start, h.hlock_range, F_RDLCK); - return ret; + return ecode; } -int tdb_delete(struct tdb_context *tdb, struct tdb_data key) +enum TDB_ERROR tdb_delete(struct tdb_context *tdb, struct tdb_data key) { tdb_off_t off; struct tdb_used_record rec; @@ -683,20 +661,17 @@ int tdb_delete(struct tdb_context *tdb, struct tdb_data key) off = find_and_lock(tdb, key, F_WRLCK, &h, &rec, NULL); if (TDB_OFF_IS_ERR(off)) { - tdb->ecode = off; - return -1; + return off; } if (!off) { - tdb_unlock_hashes(tdb, h.hlock_start, h.hlock_range, F_WRLCK); - tdb->ecode = TDB_ERR_NOEXIST; - return -1; + ecode = TDB_ERR_NOEXIST; + goto unlock; } ecode = delete_from_hash(tdb, &h); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - goto unlock_err; + goto unlock; } /* Free the deleted entry. */ @@ -706,17 +681,10 @@ int tdb_delete(struct tdb_context *tdb, struct tdb_data key) + rec_key_length(&rec) + rec_data_length(&rec) + rec_extra_padding(&rec)); - if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - goto unlock_err; - } +unlock: tdb_unlock_hashes(tdb, h.hlock_start, h.hlock_range, F_WRLCK); - return 0; - -unlock_err: - tdb_unlock_hashes(tdb, h.hlock_start, h.hlock_range, F_WRLCK); - return -1; + return ecode; } int tdb_close(struct tdb_context *tdb) @@ -759,15 +727,10 @@ int tdb_close(struct tdb_context *tdb) return ret; } -enum TDB_ERROR tdb_error(const struct tdb_context *tdb) -{ - return tdb->ecode; -} - -const char *tdb_errorstr(const struct tdb_context *tdb) +const char *tdb_errorstr(enum TDB_ERROR ecode) { /* Gcc warns if you miss a case in the switch, so use that. */ - switch (tdb->ecode) { + switch (ecode) { case TDB_SUCCESS: return "Success"; case TDB_ERR_CORRUPT: return "Corrupt database"; case TDB_ERR_IO: return "IO Error"; @@ -792,8 +755,6 @@ enum TDB_ERROR COLD tdb_logerr(struct tdb_context *tdb, /* tdb_open paths care about errno, so save it. */ int saved_errno = errno; - tdb->ecode = ecode; - if (!tdb->logfn) return ecode; diff --git a/ccan/tdb2/tdb2.h b/ccan/tdb2/tdb2.h index 4fd34e8b..c0ab8eb0 100644 --- a/ccan/tdb2/tdb2.h +++ b/ccan/tdb2/tdb2.h @@ -124,18 +124,18 @@ enum TDB_ERROR { * * This inserts (or overwrites) a key/value pair in the TDB. If flag * is TDB_REPLACE, it doesn't matter whether the key exists or not; - * TDB_INSERT means it must not exist (TDB_ERR_EXISTS otherwise), - * and TDB_MODIFY means it must exist (TDB_ERR_NOEXIST otherwise). + * TDB_INSERT means it must not exist (returns TDB_ERR_EXISTS otherwise), + * and TDB_MODIFY means it must exist (returns TDB_ERR_NOEXIST otherwise). * - * On success, this returns 0, on failure -1, and sets tdb_error(). + * On success, this returns TDB_SUCCESS. * * See also: * tdb_fetch, tdb_transaction_start, tdb_append, tdb_delete. */ -int tdb_store(struct tdb_context *tdb, - struct tdb_data key, - struct tdb_data dbuf, - int flag); +enum TDB_ERROR tdb_store(struct tdb_context *tdb, + struct tdb_data key, + struct tdb_data dbuf, + int flag); /* flags to tdb_store() */ #define TDB_REPLACE 1 /* A readability place holder */ @@ -146,44 +146,26 @@ int tdb_store(struct tdb_context *tdb, * tdb_fetch - fetch a value from a tdb. * @tdb: the tdb context returned from tdb_open() * @key: the key + * @data: pointer to data. * - * This looks up a key in the database and returns it, or returns tdb_null - * and sets tdb_error() if there's a problem (usually, TDB_ERR_NOEXIST). + * This looks up a key in the database and sets it in @data. * - * It is your responsibility to call free() on the returned structure's - * dptr. - */ -struct tdb_data tdb_fetch(struct tdb_context *tdb, struct tdb_data key); - -/** - * enum TDB_ERROR - error codes for TDB - * - * See Also: - * tdb_error(), tdb_errorstr() - */ - -/** - * tdb_error - fetch the last error value from the tdb. - * @tdb: the tdb context returned from tdb_open() + * If it returns TDB_SUCCESS, the key was found: it is your + * responsibility to call free() on @data->dptr. * - * This returns the last error, or TDB_SUCCESS. It always returns TDB_SUCCESS - * immediately after tdb_open() returns the (non-NULL) tdb context. - * - * See Also: - * tdb_errorstr() + * Otherwise, it returns an error (usually, TDB_ERR_NOEXIST) and @data is + * undefined. */ -enum TDB_ERROR tdb_error(const struct tdb_context *tdb); +enum TDB_ERROR tdb_fetch(struct tdb_context *tdb, struct tdb_data key, + struct tdb_data *data); /** * tdb_errorstr - map the tdb error onto a constant readable string - * @tdb: the tdb context returned from tdb_open() - * - * This is more useful for displaying errors to users than tdb_error. + * @ecode: the enum TDB_ERROR to map. * - * See Also: - * tdb_error() + * This is useful for displaying errors to users. */ -const char *tdb_errorstr(const struct tdb_context *tdb); +const char *tdb_errorstr(enum TDB_ERROR ecode); /** * tdb_append - append a value to a key/value pair in a tdb. @@ -196,27 +178,23 @@ const char *tdb_errorstr(const struct tdb_context *tdb); * doesn't exist, it's equivalent to tdb_store (with an additional hint that * you expect to expand the record in future). * - * Returns 0 on success, -1 on failure (and sets tdb_error()). - * * See Also: * tdb_fetch(), tdb_store() */ -int tdb_append(struct tdb_context *tdb, - struct tdb_data key, - struct tdb_data dbuf); +enum TDB_ERROR tdb_append(struct tdb_context *tdb, + struct tdb_data key, struct tdb_data dbuf); /** * tdb_delete - delete a key from a tdb. * @tdb: the tdb context returned from tdb_open() * @key: the key to delete. * - * Returns 0 on success, or -1 on error (usually tdb_error() would be - * TDB_ERR_NOEXIST in that case). + * Returns TDB_SUCCESS on success, or an error (usually TDB_ERR_NOEXIST). * * See Also: * tdb_fetch(), tdb_store() */ -int tdb_delete(struct tdb_context *tdb, struct tdb_data key); +enum TDB_ERROR tdb_delete(struct tdb_context *tdb, struct tdb_data key); /** * tdb_transaction_start - start a transaction @@ -226,12 +204,10 @@ int tdb_delete(struct tdb_context *tdb, struct tdb_data key); * to read the tdb, but not alter it (they will block), nor will they see * any changes until tdb_transaction_commit() is called. * - * On failure, returns -1 and sets tdb_error(). - * * See Also: * tdb_transaction_cancel, tdb_transaction_commit. */ -int tdb_transaction_start(struct tdb_context *tdb); +enum TDB_ERROR tdb_transaction_start(struct tdb_context *tdb); /** * tdb_transaction_cancel - abandon a transaction @@ -252,14 +228,13 @@ void tdb_transaction_cancel(struct tdb_context *tdb); * making it robust against machine crashes, but very slow compared to * other TDB operations. * - * Returns 0 on success, or -1 on failure: this can only be caused by - * unexpected errors (eg. I/O or memory); this is no point looping on - * transaction failure. + * A failure can only be caused by unexpected errors (eg. I/O or + * memory); this is no point looping on transaction failure. * * See Also: * tdb_transaction_prepare_commit() */ -int tdb_transaction_commit(struct tdb_context *tdb); +enum TDB_ERROR tdb_transaction_commit(struct tdb_context *tdb); /** * tdb_transaction_prepare_commit - prepare to commit a transaction @@ -269,12 +244,10 @@ int tdb_transaction_commit(struct tdb_context *tdb); * tdb_transaction_commit): if this succeeds then a transaction will only * fail if the write() or fsync() calls fail. * - * Returns 0 on success, or -1 on failure. - * * See Also: * tdb_transaction_commit() */ -int tdb_transaction_prepare_commit(struct tdb_context *tdb); +enum TDB_ERROR tdb_transaction_prepare_commit(struct tdb_context *tdb); /* FIXME: Make typesafe */ typedef int (*tdb_traverse_func)(struct tdb_context *, TDB_DATA, TDB_DATA, void *); @@ -294,31 +267,36 @@ typedef int (*tdb_traverse_func)(struct tdb_context *, TDB_DATA, TDB_DATA, void * current key does not undermine the reliability of the traversal. * * On success, returns the number of keys iterated. On error returns - * -1 and sets tdb_error(). + * a negative enum TDB_ERROR value. */ int64_t tdb_traverse(struct tdb_context *tdb, tdb_traverse_func fn, void *p); /** * tdb_firstkey - get the "first" key in a TDB * @tdb: the tdb context returned from tdb_open() + * @key: pointer to key. * * This returns an arbitrary key in the database; with tdb_nextkey() it allows - * open-coded traversal of the database. + * open-coded traversal of the database, though it is slightly less efficient + * than tdb_traverse. * - * On failure, returns tdb_null and sets tdb_error(). On success, returns - * a key, or tdb_null and set tdb_error() to TDB_SUCCESS for an empty database. + * It is your responsibility to free @key->dptr on success. + * + * Returns TDB_ERR_NOEXIST if the database is empty. */ -TDB_DATA tdb_firstkey(struct tdb_context *tdb); +enum TDB_ERROR tdb_firstkey(struct tdb_context *tdb, struct tdb_data *key); /** * tdb_nextkey - get the "next" key in a TDB * @tdb: the tdb context returned from tdb_open() * @key: a key returned by tdb_firstkey() or tdb_nextkey(). * - * This returns another key in the database. On failure or the last key - * it returns tdb_null: tdb_error() will be TDB_SUCCESS if it was the last key. + * This returns another key in the database; it will free @key.dptr for + * your convenience. + * + * Returns TDB_ERR_NOEXIST if there are no more keys. */ -TDB_DATA tdb_nextkey(struct tdb_context *tdb, TDB_DATA key); +enum TDB_ERROR tdb_nextkey(struct tdb_context *tdb, struct tdb_data *key); /** * tdb_chainlock - lock a record in the TDB @@ -336,14 +314,14 @@ TDB_DATA tdb_nextkey(struct tdb_context *tdb, TDB_DATA key); * See Also: * tdb_chainunlock() */ -int tdb_chainlock(struct tdb_context *tdb, TDB_DATA key); +enum TDB_ERROR tdb_chainlock(struct tdb_context *tdb, TDB_DATA key); /** * tdb_chainunlock - unlock a record in the TDB * @tdb: the tdb context returned from tdb_open() * @key: the key to unlock. */ -int tdb_chainunlock(struct tdb_context *tdb, TDB_DATA key); +enum TDB_ERROR tdb_chainunlock(struct tdb_context *tdb, TDB_DATA key); /** * tdb_check - check a TDB for consistency @@ -353,13 +331,14 @@ int tdb_chainunlock(struct tdb_context *tdb, TDB_DATA key); * * This performs a consistency check of the open database, optionally calling * a check() function on each record so you can do your own data consistency - * checks as well. If check() returns non-zero, it is considered a failure. - * - * Returns 0 on success, or -1 on failure and sets tdb_error(). + * checks as well. If check() returns an error, that is returned from + * tdb_check(). */ -int tdb_check(struct tdb_context *tdb, - int (*check)(TDB_DATA key, TDB_DATA data, void *private_data), - void *private_data); +enum TDB_ERROR tdb_check(struct tdb_context *tdb, + enum TDB_ERROR (*check)(TDB_DATA key, + TDB_DATA data, + void *private_data), + void *private_data); /** * enum tdb_summary_flags - flags for tdb_summary. @@ -372,18 +351,19 @@ enum tdb_summary_flags { * tdb_summary - return a string describing the TDB state * @tdb: the tdb context returned from tdb_open() * @flags: flags to control the summary output. + * @summary: pointer to string to allocate. * * This returns a developer-readable string describing the overall * state of the tdb, such as the percentage used and sizes of records. * It is designed to provide information about the tdb at a glance * without displaying any keys or data in the database. * - * On success, returns a nul-terminated multi-line string. On failure, - * returns NULL and sets tdb_error(). + * On success, sets @summary to point to a malloc()'ed nul-terminated + * multi-line string. It is your responsibility to free() it. */ -char *tdb_summary(struct tdb_context *tdb, enum tdb_summary_flags flags); - - +enum TDB_ERROR tdb_summary(struct tdb_context *tdb, + enum tdb_summary_flags flags, + char **summary); /** * enum tdb_attribute_type - descriminator for union tdb_attribute. diff --git a/ccan/tdb2/test/external-agent.c b/ccan/tdb2/test/external-agent.c index 35314802..b5dc56f4 100644 --- a/ccan/tdb2/test/external-agent.c +++ b/ccan/tdb2/test/external-agent.c @@ -22,6 +22,7 @@ static enum agent_return do_operation(enum operation op, const char *name) TDB_DATA k; enum agent_return ret; TDB_DATA data; + enum TDB_ERROR ecode; if (op != OPEN && !tdb) { diag("external: No tdb open!"); @@ -50,19 +51,19 @@ static enum agent_return do_operation(enum operation op, const char *name) ret = SUCCESS; break; case FETCH: - data = tdb_fetch(tdb, k); - if (data.dptr == NULL) { - if (tdb_error(tdb) == TDB_ERR_NOEXIST) - ret = FAILED; - else - ret = OTHER_FAILURE; + ecode = tdb_fetch(tdb, k, &data); + if (ecode == TDB_ERR_NOEXIST) { + ret = FAILED; + } else if (ecode < 0) { + ret = OTHER_FAILURE; } else if (data.dsize != k.dsize || memcmp(data.dptr, k.dptr, k.dsize) != 0) { ret = OTHER_FAILURE; + free(data.dptr); } else { ret = SUCCESS; + free(data.dptr); } - free(data.dptr); break; case STORE: ret = tdb_store(tdb, k, k, 0) == 0 ? SUCCESS : OTHER_FAILURE; diff --git a/ccan/tdb2/test/run-01-new_database.c b/ccan/tdb2/test/run-01-new_database.c index ee6cd55d..a2f5e8fc 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", 182, FAILTEST_MALLOC -#define LOGGING_MALLOC "tdb.c", 792, FAILTEST_MALLOC +#define INITIAL_TDB_MALLOC "tdb.c", 189, FAILTEST_MALLOC +#define LOGGING_MALLOC "tdb.c", 766, 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-10-simple-store.c b/ccan/tdb2/test/run-10-simple-store.c index 097532a2..f7d554ae 100644 --- a/ccan/tdb2/test/run-10-simple-store.c +++ b/ccan/tdb2/test/run-10-simple-store.c @@ -18,22 +18,22 @@ int main(int argc, char *argv[]) struct tdb_data key = { (unsigned char *)"key", 3 }; struct tdb_data data = { (unsigned char *)"data", 4 }; - plan_tests(sizeof(flags) / sizeof(flags[0]) * 9 + 1); + plan_tests(sizeof(flags) / sizeof(flags[0]) * 7 + 1); for (i = 0; i < sizeof(flags) / sizeof(flags[0]); i++) { tdb = tdb_open("run-10-simple-store.tdb", flags[i], O_RDWR|O_CREAT|O_TRUNC, 0600, &tap_log_attr); ok1(tdb); if (tdb) { /* Modify should fail. */ - ok1(tdb_store(tdb, key, data, TDB_MODIFY) == -1); - ok1(tdb_error(tdb) == TDB_ERR_NOEXIST); + ok1(tdb_store(tdb, key, data, TDB_MODIFY) + == TDB_ERR_NOEXIST); ok1(tdb_check(tdb, NULL, NULL) == 0); /* Insert should succeed. */ ok1(tdb_store(tdb, key, data, TDB_INSERT) == 0); ok1(tdb_check(tdb, NULL, NULL) == 0); /* Second insert should fail. */ - ok1(tdb_store(tdb, key, data, TDB_INSERT) == -1); - ok1(tdb_error(tdb) == TDB_ERR_EXISTS); + ok1(tdb_store(tdb, key, data, TDB_INSERT) + == TDB_ERR_EXISTS); ok1(tdb_check(tdb, NULL, NULL) == 0); tdb_close(tdb); } diff --git a/ccan/tdb2/test/run-11-simple-fetch.c b/ccan/tdb2/test/run-11-simple-fetch.c index 3a6ac5d9..8cd281e2 100644 --- a/ccan/tdb2/test/run-11-simple-fetch.c +++ b/ccan/tdb2/test/run-11-simple-fetch.c @@ -27,15 +27,13 @@ int main(int argc, char *argv[]) struct tdb_data d; /* fetch should fail. */ - d = tdb_fetch(tdb, key); - ok1(d.dptr == NULL); - ok1(tdb_error(tdb) == TDB_ERR_NOEXIST); + ok1(tdb_fetch(tdb, key, &d) == TDB_ERR_NOEXIST); ok1(tdb_check(tdb, NULL, NULL) == 0); /* Insert should succeed. */ ok1(tdb_store(tdb, key, data, TDB_INSERT) == 0); ok1(tdb_check(tdb, NULL, NULL) == 0); /* Fetch should now work. */ - d = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &d) == TDB_SUCCESS); ok1(data_equal(d, data)); free(d.dptr); ok1(tdb_check(tdb, NULL, NULL) == 0); diff --git a/ccan/tdb2/test/run-12-store.c b/ccan/tdb2/test/run-12-store.c index dc32d91c..2cc3e86b 100644 --- a/ccan/tdb2/test/run-12-store.c +++ b/ccan/tdb2/test/run-12-store.c @@ -39,7 +39,7 @@ int main(int argc, char *argv[]) fixed_hattr.base.next = &tap_log_attr; - plan_tests(sizeof(flags) / sizeof(flags[0]) * (1 + 500 * 2) + 1); + plan_tests(sizeof(flags) / sizeof(flags[0]) * (1 + 500 * 3) + 1); for (i = 0; i < sizeof(flags) / sizeof(flags[0]); i++) { tdb = tdb_open("run-12-store.tdb", flags[i], O_RDWR|O_CREAT|O_TRUNC, 0600, &fixed_hattr); @@ -52,7 +52,7 @@ int main(int argc, char *argv[]) for (j = 0; j < 500; j++) { struct tdb_data d; ok1(tdb_store(tdb, key, data, TDB_REPLACE) == 0); - d = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &d) == TDB_SUCCESS); ok1(equal(d, data)); free(d.dptr); } diff --git a/ccan/tdb2/test/run-13-delete.c b/ccan/tdb2/test/run-13-delete.c index 9cf5962e..6295523b 100644 --- a/ccan/tdb2/test/run-13-delete.c +++ b/ccan/tdb2/test/run-13-delete.c @@ -31,7 +31,7 @@ static bool store_records(struct tdb_context *tdb) for (i = 0; i < 1000; i++) { if (tdb_store(tdb, key, data, TDB_REPLACE) != 0) return false; - d = tdb_fetch(tdb, key); + tdb_fetch(tdb, key, &d); if (d.dsize != data.dsize) return false; if (memcmp(d.dptr, data.dptr, d.dsize) != 0) @@ -50,8 +50,7 @@ static void test_val(struct tdb_context *tdb, uint64_t val) /* Insert an entry, then delete it. */ v = val; /* Delete should fail. */ - ok1(tdb_delete(tdb, key) == -1); - ok1(tdb_error(tdb) == TDB_ERR_NOEXIST); + ok1(tdb_delete(tdb, key) == TDB_ERR_NOEXIST); ok1(tdb_check(tdb, NULL, NULL) == 0); /* Insert should succeed. */ @@ -69,11 +68,11 @@ static void test_val(struct tdb_context *tdb, uint64_t val) ok1(tdb_check(tdb, NULL, NULL) == 0); /* Can find both? */ - d = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &d) == TDB_SUCCESS); ok1(d.dsize == data.dsize); free(d.dptr); v = val; - d = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &d) == TDB_SUCCESS); ok1(d.dsize == data.dsize); free(d.dptr); @@ -93,7 +92,7 @@ static void test_val(struct tdb_context *tdb, uint64_t val) /* Can still find second? */ v = val + 1; - d = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &d) == TDB_SUCCESS); ok1(d.dsize == data.dsize); free(d.dptr); @@ -107,15 +106,15 @@ static void test_val(struct tdb_context *tdb, uint64_t val) ok1(tdb_store(tdb, key, data, TDB_INSERT) == 0); /* We can still find them all, right? */ - d = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &d) == TDB_SUCCESS); ok1(d.dsize == data.dsize); free(d.dptr); v = val + 1; - d = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &d) == TDB_SUCCESS); ok1(d.dsize == data.dsize); free(d.dptr); v = val + 2; - d = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &d) == TDB_SUCCESS); ok1(d.dsize == data.dsize); free(d.dptr); @@ -125,11 +124,11 @@ static void test_val(struct tdb_context *tdb, uint64_t val) ok1(tdb_check(tdb, NULL, NULL) == 0); v = val; - d = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &d) == TDB_SUCCESS); ok1(d.dsize == data.dsize); free(d.dptr); v = val + 2; - d = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &d) == TDB_SUCCESS); ok1(d.dsize == data.dsize); free(d.dptr); @@ -163,7 +162,7 @@ int main(int argc, char *argv[]) fixed_hattr.base.next = &tap_log_attr; plan_tests(sizeof(flags) / sizeof(flags[0]) - * (32 * 3 + 5 + sizeof(vals)/sizeof(vals[0])*2) + 1); + * (39 * 3 + 5 + sizeof(vals)/sizeof(vals[0])*2) + 1); for (i = 0; i < sizeof(flags) / sizeof(flags[0]); i++) { tdb = tdb_open("run-13-delete.tdb", flags[i], O_RDWR|O_CREAT|O_TRUNC, 0600, &clash_hattr); diff --git a/ccan/tdb2/test/run-15-append.c b/ccan/tdb2/test/run-15-append.c index 36b6162f..d2481658 100644 --- a/ccan/tdb2/test/run-15-append.c +++ b/ccan/tdb2/test/run-15-append.c @@ -42,7 +42,7 @@ int main(int argc, char *argv[]) buffer[i] = i; plan_tests(sizeof(flags) / sizeof(flags[0]) - * ((3 + MAX_SIZE/SIZE_STEP * 4) * 2 + 6) + * ((3 + MAX_SIZE/SIZE_STEP * 5) * 2 + 7) + 1); /* Using tdb_store. */ @@ -59,7 +59,7 @@ int main(int argc, char *argv[]) data.dsize = j; ok1(tdb_store(tdb, key, data, TDB_REPLACE) == 0); ok1(tdb_check(tdb, NULL, NULL) == 0); - data = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &data) == TDB_SUCCESS); ok1(data.dsize == j); ok1(memcmp(data.dptr, buffer, data.dsize) == 0); free(data.dptr); @@ -89,7 +89,7 @@ int main(int argc, char *argv[]) data.dsize = j - prev_len; ok1(tdb_append(tdb, key, data) == 0); ok1(tdb_check(tdb, NULL, NULL) == 0); - data = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &data) == TDB_SUCCESS); ok1(data.dsize == j); ok1(memcmp(data.dptr, buffer, data.dsize) == 0); free(data.dptr); @@ -117,7 +117,7 @@ int main(int argc, char *argv[]) data.dsize = MAX_SIZE; ok1(tdb_append(tdb, key, data) == 0); ok1(tdb_check(tdb, NULL, NULL) == 0); - data = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &data) == TDB_SUCCESS); ok1(data.dsize == MAX_SIZE); ok1(memcmp(data.dptr, buffer, data.dsize) == 0); free(data.dptr); diff --git a/ccan/tdb2/test/run-25-hashoverload.c b/ccan/tdb2/test/run-25-hashoverload.c index 42d49de7..fced3f30 100644 --- a/ccan/tdb2/test/run-25-hashoverload.c +++ b/ccan/tdb2/test/run-25-hashoverload.c @@ -36,7 +36,7 @@ int main(int argc, char *argv[]) hattr.base.next = &tap_log_attr; - plan_tests(5395); + plan_tests(6883); for (i = 0; i < sizeof(flags) / sizeof(flags[0]); i++) { struct tdb_data d; @@ -58,7 +58,7 @@ int main(int argc, char *argv[]) /* Check we can find them all. */ for (j = 0; j < (1 << TDB_HASH_GROUP_BITS) + 1; j++) { - d = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &d) == TDB_SUCCESS); ok1(d.dsize == sizeof(j)); ok1(d.dptr != NULL); ok1(d.dptr && memcmp(d.dptr, &j, d.dsize) == 0); @@ -70,7 +70,7 @@ int main(int argc, char *argv[]) j < (16 << TDB_HASH_GROUP_BITS); j++) { ok1(tdb_store(tdb, key, dbuf, TDB_INSERT) == 0); - d = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &d) == TDB_SUCCESS); ok1(d.dsize == sizeof(j)); ok1(d.dptr != NULL); ok1(d.dptr && memcmp(d.dptr, &j, d.dsize) == 0); @@ -90,7 +90,7 @@ int main(int argc, char *argv[]) for (j = (1 << TDB_HASH_GROUP_BITS); j < (16 << TDB_HASH_GROUP_BITS); j++) { - d = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &d) == TDB_SUCCESS); ok1(d.dsize == sizeof(j)); ok1(d.dptr != NULL); ok1(d.dptr && memcmp(d.dptr, &j, d.dsize) == 0); diff --git a/ccan/tdb2/test/run-55-transaction.c b/ccan/tdb2/test/run-55-transaction.c index fe80a358..4ebf792a 100644 --- a/ccan/tdb2/test/run-55-transaction.c +++ b/ccan/tdb2/test/run-55-transaction.c @@ -22,7 +22,7 @@ int main(int argc, char *argv[]) for (i = 0; i < 1000; i++) buffer[i] = i; - plan_tests(sizeof(flags) / sizeof(flags[0]) * 18 + 1); + plan_tests(sizeof(flags) / sizeof(flags[0]) * 20 + 1); for (i = 0; i < sizeof(flags) / sizeof(flags[0]); i++) { tdb = tdb_open("run-55-transaction.tdb", flags[i], @@ -35,7 +35,7 @@ int main(int argc, char *argv[]) data.dptr = buffer; data.dsize = 1000; ok1(tdb_store(tdb, key, data, TDB_INSERT) == 0); - data = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &data) == TDB_SUCCESS); ok1(data.dsize == 1000); ok1(memcmp(data.dptr, buffer, data.dsize) == 0); free(data.dptr); @@ -44,23 +44,21 @@ int main(int argc, char *argv[]) tdb_transaction_cancel(tdb); ok1(tdb->allrecord_lock.count == 0 && tdb->num_lockrecs == 0); ok1(tdb_check(tdb, NULL, NULL) == 0); - data = tdb_fetch(tdb, key); - ok1(data.dsize == 0); - ok1(data.dptr == NULL); + ok1(tdb_fetch(tdb, key, &data) == TDB_ERR_NOEXIST); /* Commit the transaction. */ ok1(tdb_transaction_start(tdb) == 0); data.dptr = buffer; data.dsize = 1000; ok1(tdb_store(tdb, key, data, TDB_INSERT) == 0); - data = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &data) == TDB_SUCCESS); ok1(data.dsize == 1000); ok1(memcmp(data.dptr, buffer, data.dsize) == 0); free(data.dptr); ok1(tdb_transaction_commit(tdb) == 0); ok1(tdb->allrecord_lock.count == 0 && tdb->num_lockrecs == 0); ok1(tdb_check(tdb, NULL, NULL) == 0); - data = tdb_fetch(tdb, key); + ok1(tdb_fetch(tdb, key, &data) == TDB_SUCCESS); ok1(data.dsize == 1000); ok1(memcmp(data.dptr, buffer, data.dsize) == 0); free(data.dptr); diff --git a/ccan/tdb2/test/run-57-die-during-transaction.c b/ccan/tdb2/test/run-57-die-during-transaction.c index 793636a9..f9b0426e 100644 --- a/ccan/tdb2/test/run-57-die-during-transaction.c +++ b/ccan/tdb2/test/run-57-die-during-transaction.c @@ -145,6 +145,10 @@ reset: unlink(TEST_DBNAME); tdb = tdb_open(TEST_DBNAME, TDB_NOMMAP, O_CREAT|O_TRUNC|O_RDWR, 0600, &tap_log_attr); + if (!tdb) { + diag("Failed opening TDB: %s", strerror(errno)); + return false; + } if (setjmp(jmpbuf) != 0) { /* We're partway through. Simulate our death. */ diff --git a/ccan/tdb2/test/run-firstkey-nextkey.c b/ccan/tdb2/test/run-firstkey-nextkey.c index a71e1a7a..b8a12bd3 100644 --- a/ccan/tdb2/test/run-firstkey-nextkey.c +++ b/ccan/tdb2/test/run-firstkey-nextkey.c @@ -38,14 +38,24 @@ static int trav(struct tdb_context *tdb, TDB_DATA key, TDB_DATA dbuf, void *p) return 0; } +/* Since tdb_nextkey frees dptr, we need to clone it. */ +static TDB_DATA dup_key(TDB_DATA key) +{ + void *p = malloc(key.dsize); + memcpy(p, key.dptr, key.dsize); + key.dptr = p; + return key; +} + int main(int argc, char *argv[]) { unsigned int i, j; int num; struct trav_data td; - TDB_DATA k, k2; + TDB_DATA k; struct tdb_context *tdb; union tdb_attribute seed_attr; + enum TDB_ERROR ecode; int flags[] = { TDB_INTERNAL, TDB_DEFAULT, TDB_NOMMAP, TDB_INTERNAL|TDB_CONVERT, TDB_CONVERT, @@ -56,7 +66,7 @@ int main(int argc, char *argv[]) seed_attr.seed.seed = 6334326220117065685ULL; plan_tests(sizeof(flags) / sizeof(flags[0]) - * (NUM_RECORDS*4 + (NUM_RECORDS-1)*2 + 20) + 1); + * (NUM_RECORDS*6 + (NUM_RECORDS-1)*3 + 22) + 1); for (i = 0; i < sizeof(flags) / sizeof(flags[0]); i++) { tdb = tdb_open("run-traverse.tdb", flags[i], O_RDWR|O_CREAT|O_TRUNC, 0600, &seed_attr); @@ -64,39 +74,33 @@ int main(int argc, char *argv[]) if (!tdb) continue; - ok1(tdb_firstkey(tdb).dptr == NULL); - ok1(tdb_error(tdb) == TDB_SUCCESS); + ok1(tdb_firstkey(tdb, &k) == TDB_ERR_NOEXIST); /* One entry... */ k.dptr = (unsigned char *)# k.dsize = sizeof(num); num = 0; ok1(tdb_store(tdb, k, k, TDB_INSERT) == 0); - k = tdb_firstkey(tdb); + ok1(tdb_firstkey(tdb, &k) == TDB_SUCCESS); ok1(k.dsize == sizeof(num)); ok1(memcmp(k.dptr, &num, sizeof(num)) == 0); - k2 = tdb_nextkey(tdb, k); - ok1(k2.dsize == 0 && k2.dptr == NULL); - free(k.dptr); + ok1(tdb_nextkey(tdb, &k) == TDB_ERR_NOEXIST); /* Two entries. */ k.dptr = (unsigned char *)# k.dsize = sizeof(num); num = 1; ok1(tdb_store(tdb, k, k, TDB_INSERT) == 0); - k = tdb_firstkey(tdb); + ok1(tdb_firstkey(tdb, &k) == TDB_SUCCESS); ok1(k.dsize == sizeof(num)); memcpy(&num, k.dptr, sizeof(num)); ok1(num == 0 || num == 1); - k2 = tdb_nextkey(tdb, k); - ok1(k2.dsize == sizeof(j)); - free(k.dptr); - memcpy(&j, k2.dptr, sizeof(j)); + ok1(tdb_nextkey(tdb, &k) == TDB_SUCCESS); + ok1(k.dsize == sizeof(j)); + memcpy(&j, k.dptr, sizeof(j)); ok1(j == 0 || j == 1); ok1(j != num); - k = tdb_nextkey(tdb, k2); - ok1(k.dsize == 0 && k.dptr == NULL); - free(k2.dptr); + ok1(tdb_nextkey(tdb, &k) == TDB_ERR_NOEXIST); /* Clean up. */ k.dptr = (unsigned char *)# @@ -115,34 +119,35 @@ int main(int argc, char *argv[]) ok1(td.calls == NUM_RECORDS); /* Simple loop should match tdb_traverse */ - for (j = 0, k = tdb_firstkey(tdb); j < td.calls; j++) { + for (j = 0, ecode = tdb_firstkey(tdb, &k); j < td.calls; j++) { int val; + ok1(ecode == TDB_SUCCESS); ok1(k.dsize == sizeof(val)); memcpy(&val, k.dptr, k.dsize); ok1(td.records[j] == val); - k2 = tdb_nextkey(tdb, k); - free(k.dptr); - k = k2; + ecode = tdb_nextkey(tdb, &k); } /* But arbitrary orderings should work too. */ for (j = td.calls-1; j > 0; j--) { k.dptr = (unsigned char *)&td.records[j-1]; k.dsize = sizeof(td.records[j-1]); - k = tdb_nextkey(tdb, k); + k = dup_key(k); + ok1(tdb_nextkey(tdb, &k) == TDB_SUCCESS); ok1(k.dsize == sizeof(td.records[j])); ok1(memcmp(k.dptr, &td.records[j], k.dsize) == 0); free(k.dptr); } /* Even delete should work. */ - for (j = 0, k = tdb_firstkey(tdb); k.dptr; j++) { + for (j = 0, ecode = tdb_firstkey(tdb, &k); + ecode != TDB_ERR_NOEXIST; + j++) { + ok1(ecode == TDB_SUCCESS); ok1(k.dsize == 4); ok1(tdb_delete(tdb, k) == 0); - k2 = tdb_nextkey(tdb, k); - free(k.dptr); - k = k2; + ecode = tdb_nextkey(tdb, &k); } diag("delete using first/nextkey gave %u of %u records", diff --git a/ccan/tdb2/test/run-simple-delete.c b/ccan/tdb2/test/run-simple-delete.c index deb7ebf2..d9d0fb1a 100644 --- a/ccan/tdb2/test/run-simple-delete.c +++ b/ccan/tdb2/test/run-simple-delete.c @@ -18,15 +18,14 @@ int main(int argc, char *argv[]) struct tdb_data key = { (unsigned char *)"key", 3 }; struct tdb_data data = { (unsigned char *)"data", 4 }; - plan_tests(sizeof(flags) / sizeof(flags[0]) * 8 + 1); + plan_tests(sizeof(flags) / sizeof(flags[0]) * 7 + 1); for (i = 0; i < sizeof(flags) / sizeof(flags[0]); i++) { tdb = tdb_open("run-simple-delete.tdb", flags[i], O_RDWR|O_CREAT|O_TRUNC, 0600, &tap_log_attr); ok1(tdb); if (tdb) { /* Delete should fail. */ - ok1(tdb_delete(tdb, key) == -1); - ok1(tdb_error(tdb) == TDB_ERR_NOEXIST); + ok1(tdb_delete(tdb, key) == TDB_ERR_NOEXIST); ok1(tdb_check(tdb, NULL, NULL) == 0); /* Insert should succeed. */ ok1(tdb_store(tdb, key, data, TDB_INSERT) == 0); diff --git a/ccan/tdb2/test/run-summary.c b/ccan/tdb2/test/run-summary.c index 8ef5c80e..24d9428e 100644 --- a/ccan/tdb2/test/run-summary.c +++ b/ccan/tdb2/test/run-summary.c @@ -20,7 +20,7 @@ int main(int argc, char *argv[]) struct tdb_data data = { (unsigned char *)&j, sizeof(j) }; char *summary; - plan_tests(sizeof(flags) / sizeof(flags[0]) * (1 + 2 * 4) + 1); + plan_tests(sizeof(flags) / sizeof(flags[0]) * (1 + 2 * 5) + 1); for (i = 0; i < sizeof(flags) / sizeof(flags[0]); i++) { tdb = tdb_open("run-summary.tdb", flags[i], O_RDWR|O_CREAT|O_TRUNC, 0600, &tap_log_attr); @@ -39,7 +39,7 @@ int main(int argc, char *argv[]) for (j = 0; j <= TDB_SUMMARY_HISTOGRAMS; j += TDB_SUMMARY_HISTOGRAMS) { - summary = tdb_summary(tdb, j); + ok1(tdb_summary(tdb, j, &summary) == TDB_SUCCESS); ok1(strstr(summary, "Number of records: 500\n")); ok1(strstr(summary, "Smallest/average/largest keys: 4/4/4\n")); ok1(strstr(summary, "Smallest/average/largest data: 0/2/4\n")); diff --git a/ccan/tdb2/test/run-tdb_errorstr.c b/ccan/tdb2/test/run-tdb_errorstr.c index e8600e7f..8a7e378e 100644 --- a/ccan/tdb2/test/run-tdb_errorstr.c +++ b/ccan/tdb2/test/run-tdb_errorstr.c @@ -9,56 +9,50 @@ int main(int argc, char *argv[]) { - struct tdb_context *tdb; + enum TDB_ERROR err; + plan_tests(TDB_ERR_RDONLY*-1 + 2); - plan_tests(1 + TDB_ERR_RDONLY*-1 + 2); - tdb = tdb_open("run-tdb_errorstr.tdb", TDB_DEFAULT, - O_RDWR|O_CREAT|O_TRUNC, 0600, NULL); - ok1(tdb); - if (tdb) { - enum TDB_ERROR err; - for (err = TDB_SUCCESS; err >= TDB_ERR_RDONLY; err--) { - tdb->ecode = err; - switch (err) { - case TDB_SUCCESS: - ok1(!strcmp(tdb_errorstr(tdb), - "Success")); - break; - case TDB_ERR_IO: - ok1(!strcmp(tdb_errorstr(tdb), - "IO Error")); - break; - case TDB_ERR_LOCK: - ok1(!strcmp(tdb_errorstr(tdb), - "Locking error")); - break; - case TDB_ERR_OOM: - ok1(!strcmp(tdb_errorstr(tdb), - "Out of memory")); - break; - case TDB_ERR_EXISTS: - ok1(!strcmp(tdb_errorstr(tdb), - "Record exists")); - break; - case TDB_ERR_EINVAL: - ok1(!strcmp(tdb_errorstr(tdb), - "Invalid parameter")); - break; - case TDB_ERR_NOEXIST: - ok1(!strcmp(tdb_errorstr(tdb), - "Record does not exist")); - break; - case TDB_ERR_RDONLY: - ok1(!strcmp(tdb_errorstr(tdb), - "write not permitted")); - break; - case TDB_ERR_CORRUPT: - ok1(!strcmp(tdb_errorstr(tdb), - "Corrupt database")); - } + for (err = TDB_SUCCESS; err >= TDB_ERR_RDONLY; err--) { + switch (err) { + case TDB_SUCCESS: + ok1(!strcmp(tdb_errorstr(err), + "Success")); + break; + case TDB_ERR_IO: + ok1(!strcmp(tdb_errorstr(err), + "IO Error")); + break; + case TDB_ERR_LOCK: + ok1(!strcmp(tdb_errorstr(err), + "Locking error")); + break; + case TDB_ERR_OOM: + ok1(!strcmp(tdb_errorstr(err), + "Out of memory")); + break; + case TDB_ERR_EXISTS: + ok1(!strcmp(tdb_errorstr(err), + "Record exists")); + break; + case TDB_ERR_EINVAL: + ok1(!strcmp(tdb_errorstr(err), + "Invalid parameter")); + break; + case TDB_ERR_NOEXIST: + ok1(!strcmp(tdb_errorstr(err), + "Record does not exist")); + break; + case TDB_ERR_RDONLY: + ok1(!strcmp(tdb_errorstr(err), + "write not permitted")); + break; + case TDB_ERR_CORRUPT: + ok1(!strcmp(tdb_errorstr(err), + "Corrupt database")); + break; } - tdb->ecode = err; - ok1(!strcmp(tdb_errorstr(tdb), "Invalid error code")); } + ok1(!strcmp(tdb_errorstr(err), "Invalid error code")); + return exit_status(); } diff --git a/ccan/tdb2/test/run-traverse.c b/ccan/tdb2/test/run-traverse.c index 5d35db48..0a631002 100644 --- a/ccan/tdb2/test/run-traverse.c +++ b/ccan/tdb2/test/run-traverse.c @@ -56,8 +56,8 @@ static int trav(struct tdb_context *tdb, TDB_DATA key, TDB_DATA dbuf, void *p) td->high = val; if (td->delete) { - if (tdb_delete(tdb, key) != 0) { - td->delete_error = tdb_error(tdb); + td->delete_error = tdb_delete(tdb, key); + if (td->delete_error != TDB_SUCCESS) { return -1; } } @@ -95,8 +95,8 @@ static int trav_grow(struct tdb_context *tdb, TDB_DATA key, TDB_DATA dbuf, /* Make a big difference to the database. */ dbuf.dptr = buffer; dbuf.dsize = sizeof(buffer); - if (tdb_append(tdb, key, dbuf) != 0) { - tgd->error = tdb_error(tdb); + tgd->error = tdb_append(tdb, key, dbuf); + if (tgd->error != TDB_SUCCESS) { return -1; } return 0; diff --git a/ccan/tdb2/transaction.c b/ccan/tdb2/transaction.c index 66565d67..01f0713a 100644 --- a/ccan/tdb2/transaction.c +++ b/ccan/tdb2/transaction.c @@ -513,42 +513,38 @@ static void _tdb_transaction_cancel(struct tdb_context *tdb) start a tdb transaction. No token is returned, as only a single transaction is allowed to be pending per tdb_context */ -int tdb_transaction_start(struct tdb_context *tdb) +enum TDB_ERROR tdb_transaction_start(struct tdb_context *tdb) { enum TDB_ERROR ecode; /* some sanity checks */ if (tdb->read_only || (tdb->flags & TDB_INTERNAL)) { - tdb_logerr(tdb, TDB_ERR_EINVAL, TDB_LOG_USE_ERROR, - "tdb_transaction_start: cannot start a transaction" - " on a read-only or internal db"); - return -1; + return tdb_logerr(tdb, TDB_ERR_EINVAL, TDB_LOG_USE_ERROR, + "tdb_transaction_start: cannot start a" + " transaction on a read-only or internal db"); } /* cope with nested tdb_transaction_start() calls */ if (tdb->transaction != NULL) { - tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_USE_ERROR, - "tdb_transaction_start:" - " already inside transaction"); - return -1; + return tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_USE_ERROR, + "tdb_transaction_start:" + " already inside transaction"); } if (tdb_has_hash_locks(tdb)) { /* the caller must not have any locks when starting a transaction as otherwise we'll be screwed by lack of nested locks in posix */ - tdb_logerr(tdb, TDB_ERR_LOCK, TDB_LOG_USE_ERROR, - "tdb_transaction_start: cannot start a transaction" - " with locks held"); - return -1; + return tdb_logerr(tdb, TDB_ERR_LOCK, TDB_LOG_USE_ERROR, + "tdb_transaction_start: cannot start a" + " transaction with locks held"); } tdb->transaction = (struct tdb_transaction *) calloc(sizeof(struct tdb_transaction), 1); if (tdb->transaction == NULL) { - tdb_logerr(tdb, TDB_ERR_OOM, TDB_LOG_ERROR, - "tdb_transaction_start: cannot allocate"); - return -1; + return tdb_logerr(tdb, TDB_ERR_OOM, TDB_LOG_ERROR, + "tdb_transaction_start: cannot allocate"); } /* get the transaction write lock. This is a blocking lock. As @@ -556,17 +552,15 @@ int tdb_transaction_start(struct tdb_context *tdb) make this async, which we will probably do in the future */ ecode = tdb_transaction_lock(tdb, F_WRLCK); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; SAFE_FREE(tdb->transaction->blocks); SAFE_FREE(tdb->transaction); - return -1; + return ecode; } /* get a read lock over entire file. This is upgraded to a write lock during the commit */ ecode = tdb_allrecord_lock(tdb, F_RDLCK, TDB_LOCK_WAIT, true); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; goto fail_allrecord_lock; } @@ -579,13 +573,13 @@ int tdb_transaction_start(struct tdb_context *tdb) transaction specific methods */ tdb->transaction->io_methods = tdb->methods; tdb->methods = &transaction_methods; - return 0; + return TDB_SUCCESS; fail_allrecord_lock: tdb_transaction_unlock(tdb, F_WRLCK); SAFE_FREE(tdb->transaction->blocks); SAFE_FREE(tdb->transaction); - return -1; + return ecode; } @@ -971,46 +965,42 @@ static enum TDB_ERROR _tdb_transaction_prepare_commit(struct tdb_context *tdb) /* prepare to commit the current transaction */ -int tdb_transaction_prepare_commit(struct tdb_context *tdb) +enum TDB_ERROR tdb_transaction_prepare_commit(struct tdb_context *tdb) { - tdb->ecode = _tdb_transaction_prepare_commit(tdb); - if (tdb->ecode != TDB_SUCCESS) - return -1; - return 0; + return _tdb_transaction_prepare_commit(tdb); } /* commit the current transaction */ -int tdb_transaction_commit(struct tdb_context *tdb) +enum TDB_ERROR tdb_transaction_commit(struct tdb_context *tdb) { const struct tdb_methods *methods; int i; enum TDB_ERROR ecode; if (tdb->transaction == NULL) { - tdb_logerr(tdb, TDB_ERR_EINVAL, TDB_LOG_USE_ERROR, - "tdb_transaction_commit: no transaction"); - return -1; + return tdb_logerr(tdb, TDB_ERR_EINVAL, TDB_LOG_USE_ERROR, + "tdb_transaction_commit: no transaction"); } tdb_trace(tdb, "tdb_transaction_commit"); if (tdb->transaction->nesting != 0) { tdb->transaction->nesting--; - return 0; + return TDB_SUCCESS; } /* check for a null transaction */ if (tdb->transaction->blocks == NULL) { _tdb_transaction_cancel(tdb); - return 0; + return TDB_SUCCESS; } if (!tdb->transaction->prepared) { - tdb->ecode = _tdb_transaction_prepare_commit(tdb); - if (tdb->ecode != TDB_SUCCESS) - return -1; + ecode = _tdb_transaction_prepare_commit(tdb); + if (ecode != TDB_SUCCESS) + return ecode; } methods = tdb->transaction->io_methods; @@ -1045,7 +1035,7 @@ int tdb_transaction_commit(struct tdb_context *tdb) _tdb_transaction_cancel(tdb); - return -1; + return ecode; } SAFE_FREE(tdb->transaction->blocks[i]); } @@ -1056,8 +1046,7 @@ int tdb_transaction_commit(struct tdb_context *tdb) /* ensure the new data is on disk */ ecode = transaction_sync(tdb, 0, tdb->map_size); if (ecode != TDB_SUCCESS) { - tdb->ecode = ecode; - return -1; + return ecode; } /* @@ -1079,7 +1068,7 @@ int tdb_transaction_commit(struct tdb_context *tdb) transaction locks */ _tdb_transaction_cancel(tdb); - return 0; + return TDB_SUCCESS; } diff --git a/ccan/tdb2/traverse.c b/ccan/tdb2/traverse.c index 06f0bc68..5217f63b 100644 --- a/ccan/tdb2/traverse.c +++ b/ccan/tdb2/traverse.c @@ -34,55 +34,37 @@ int64_t tdb_traverse(struct tdb_context *tdb, tdb_traverse_func fn, void *p) count++; if (fn && fn(tdb, k, d, p)) { free(k.dptr); - break; + return count; } free(k.dptr); } if (ecode != TDB_ERR_NOEXIST) { - tdb->ecode = ecode; - return -1; + return ecode; } return count; } -TDB_DATA tdb_firstkey(struct tdb_context *tdb) +enum TDB_ERROR tdb_firstkey(struct tdb_context *tdb, struct tdb_data *key) { struct traverse_info tinfo; - struct tdb_data k; - enum TDB_ERROR ecode; - ecode = first_in_hash(tdb, &tinfo, &k, NULL); - if (ecode == TDB_SUCCESS) { - return k; - } - if (ecode == TDB_ERR_NOEXIST) - ecode = TDB_SUCCESS; - tdb->ecode = ecode; - return tdb_null; + return first_in_hash(tdb, &tinfo, key, NULL); } /* We lock twice, not very efficient. We could keep last key & tinfo cached. */ -TDB_DATA tdb_nextkey(struct tdb_context *tdb, TDB_DATA key) +enum TDB_ERROR tdb_nextkey(struct tdb_context *tdb, struct tdb_data *key) { struct traverse_info tinfo; struct hash_info h; struct tdb_used_record rec; - enum TDB_ERROR ecode; - tinfo.prev = find_and_lock(tdb, key, F_RDLCK, &h, &rec, &tinfo); + tinfo.prev = find_and_lock(tdb, *key, F_RDLCK, &h, &rec, &tinfo); + free(key->dptr); if (TDB_OFF_IS_ERR(tinfo.prev)) { - tdb->ecode = tinfo.prev; - return tdb_null; + return tinfo.prev; } tdb_unlock_hashes(tdb, h.hlock_start, h.hlock_range, F_RDLCK); - ecode = next_in_hash(tdb, &tinfo, &key, NULL); - if (ecode == TDB_SUCCESS) { - return key; - } - if (ecode == TDB_ERR_NOEXIST) - ecode = TDB_SUCCESS; - tdb->ecode = ecode; - return tdb_null; + return next_in_hash(tdb, &tinfo, key, NULL); } -- 2.39.2