From f5e9ed1ea965827f29fe0fa77a32e09737e51b45 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 18 Jun 2012 22:30:27 +0930 Subject: ntdb: remove ntdb_error() It was a hack to make compatibility easier. Since we're not doing that, it can go away: all callers must use the return value now. Signed-off-by: Rusty Russell --- lib/ntdb/ABI/ntdb-0.9.sigs | 1 - lib/ntdb/check.c | 6 +-- lib/ntdb/doc/TDB_porting.txt | 3 +- lib/ntdb/hash.c | 7 ++-- lib/ntdb/ntdb.c | 81 +++++++++++++++-------------------------- lib/ntdb/ntdb.h | 11 ------ lib/ntdb/open.c | 55 +++++++++++++--------------- lib/ntdb/private.h | 3 -- lib/ntdb/pyntdb.c | 2 - lib/ntdb/summary.c | 6 +-- lib/ntdb/test/failtest_helper.h | 2 +- lib/ntdb/tools/ntdbtorture.c | 69 ++++++++++++++++++++--------------- lib/ntdb/transaction.c | 74 +++++++++++++++++-------------------- lib/ntdb/traverse.c | 14 +++---- 14 files changed, 144 insertions(+), 190 deletions(-) diff --git a/lib/ntdb/ABI/ntdb-0.9.sigs b/lib/ntdb/ABI/ntdb-0.9.sigs index 6dae18fb6c..6b12ddbde7 100644 --- a/lib/ntdb/ABI/ntdb-0.9.sigs +++ b/lib/ntdb/ABI/ntdb-0.9.sigs @@ -7,7 +7,6 @@ ntdb_chainunlock_read: void (struct ntdb_context *, NTDB_DATA) ntdb_check_: enum NTDB_ERROR (struct ntdb_context *, enum NTDB_ERROR (*)(NTDB_DATA, NTDB_DATA, void *), void *) ntdb_close: int (struct ntdb_context *) ntdb_delete: enum NTDB_ERROR (struct ntdb_context *, NTDB_DATA) -ntdb_error: enum NTDB_ERROR (struct ntdb_context *) ntdb_errorstr: const char *(enum NTDB_ERROR) ntdb_exists: bool (struct ntdb_context *, NTDB_DATA) ntdb_fd: int (const struct ntdb_context *) diff --git a/lib/ntdb/check.c b/lib/ntdb/check.c index 1c676c7d45..cff6e0345e 100644 --- a/lib/ntdb/check.c +++ b/lib/ntdb/check.c @@ -812,13 +812,13 @@ _PUBLIC_ enum NTDB_ERROR ntdb_check_(struct ntdb_context *ntdb, ecode = ntdb_allrecord_lock(ntdb, F_RDLCK, NTDB_LOCK_WAIT, false); if (ecode != NTDB_SUCCESS) { - return ntdb->last_error = ecode; + return ecode; } ecode = ntdb_lock_expand(ntdb, F_RDLCK); if (ecode != NTDB_SUCCESS) { ntdb_allrecord_unlock(ntdb, F_RDLCK); - return ntdb->last_error = ecode; + return ecode; } ecode = check_header(ntdb, &recovery, &features, &num_capabilities); @@ -860,5 +860,5 @@ out: ntdb_unlock_expand(ntdb, F_RDLCK); free(fr); free(used); - return ntdb->last_error = ecode; + return ecode; } diff --git a/lib/ntdb/doc/TDB_porting.txt b/lib/ntdb/doc/TDB_porting.txt index 8df137416d..34e536ffcb 100644 --- a/lib/ntdb/doc/TDB_porting.txt +++ b/lib/ntdb/doc/TDB_porting.txt @@ -9,8 +9,7 @@ Interface differences between TDB and NTDB. - ntdb functions return NTDB_SUCCESS (ie 0) on success, and a negative error on failure, whereas tdb functions returned 0 on success, and -1 on failure. tdb then used tdb_error() to determine the error; - this is also supported in ntdb to ease backwards compatibility, - though the other form is preferred. + this API is nasty if we ever want to support threads, so is not supported. - ntdb's ntdb_fetch() returns an error, tdb's returned the data directly (or tdb_null, and you were supposed to check tdb_error() to find out why). diff --git a/lib/ntdb/hash.c b/lib/ntdb/hash.c index 95b98c0736..e87705b123 100644 --- a/lib/ntdb/hash.c +++ b/lib/ntdb/hash.c @@ -853,8 +853,7 @@ static enum NTDB_ERROR chainlock(struct ntdb_context *ntdb, const NTDB_DATA *key contention - it cannot guarantee how many records will be locked */ _PUBLIC_ enum NTDB_ERROR ntdb_chainlock(struct ntdb_context *ntdb, NTDB_DATA key) { - return ntdb->last_error = chainlock(ntdb, &key, F_WRLCK, NTDB_LOCK_WAIT, - "ntdb_chainlock"); + return chainlock(ntdb, &key, F_WRLCK, NTDB_LOCK_WAIT, "ntdb_chainlock"); } _PUBLIC_ void ntdb_chainunlock(struct ntdb_context *ntdb, NTDB_DATA key) @@ -874,8 +873,8 @@ _PUBLIC_ void ntdb_chainunlock(struct ntdb_context *ntdb, NTDB_DATA key) _PUBLIC_ enum NTDB_ERROR ntdb_chainlock_read(struct ntdb_context *ntdb, NTDB_DATA key) { - return ntdb->last_error = chainlock(ntdb, &key, F_RDLCK, NTDB_LOCK_WAIT, - "ntdb_chainlock_read"); + return chainlock(ntdb, &key, F_RDLCK, NTDB_LOCK_WAIT, + "ntdb_chainlock_read"); } _PUBLIC_ void ntdb_chainunlock_read(struct ntdb_context *ntdb, NTDB_DATA key) diff --git a/lib/ntdb/ntdb.c b/lib/ntdb/ntdb.c index 9f1e32793a..df9fe709b2 100644 --- a/lib/ntdb/ntdb.c +++ b/lib/ntdb/ntdb.c @@ -118,7 +118,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_store(struct ntdb_context *ntdb, off = find_and_lock(ntdb, key, F_WRLCK, &h, &rec, NULL); if (NTDB_OFF_IS_ERR(off)) { - return ntdb->last_error = NTDB_OFF_TO_ERR(off); + return NTDB_OFF_TO_ERR(off); } /* Now we have lock on this hash bucket. */ @@ -148,7 +148,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_store(struct ntdb_context *ntdb, } ntdb_unlock_hashes(ntdb, h.hlock_start, h.hlock_range, F_WRLCK); - return ntdb->last_error = NTDB_SUCCESS; + return NTDB_SUCCESS; } } else { if (flag == NTDB_MODIFY) { @@ -165,7 +165,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_store(struct ntdb_context *ntdb, ecode = replace_data(ntdb, &h, key, dbuf, off, old_room, off); out: ntdb_unlock_hashes(ntdb, h.hlock_start, h.hlock_range, F_WRLCK); - return ntdb->last_error = ecode; + return ecode; } _PUBLIC_ enum NTDB_ERROR ntdb_append(struct ntdb_context *ntdb, @@ -181,7 +181,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_append(struct ntdb_context *ntdb, off = find_and_lock(ntdb, key, F_WRLCK, &h, &rec, NULL); if (NTDB_OFF_IS_ERR(off)) { - return ntdb->last_error = NTDB_OFF_TO_ERR(off); + return NTDB_OFF_TO_ERR(off); } if (off) { @@ -233,7 +233,7 @@ out_free_newdata: free(newdata); out: ntdb_unlock_hashes(ntdb, h.hlock_start, h.hlock_range, F_WRLCK); - return ntdb->last_error = ecode; + return ecode; } _PUBLIC_ enum NTDB_ERROR ntdb_fetch(struct ntdb_context *ntdb, NTDB_DATA key, @@ -246,7 +246,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_fetch(struct ntdb_context *ntdb, NTDB_DATA key, off = find_and_lock(ntdb, key, F_RDLCK, &h, &rec, NULL); if (NTDB_OFF_IS_ERR(off)) { - return ntdb->last_error = NTDB_OFF_TO_ERR(off); + return NTDB_OFF_TO_ERR(off); } if (!off) { @@ -262,7 +262,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_fetch(struct ntdb_context *ntdb, NTDB_DATA key, } ntdb_unlock_hashes(ntdb, h.hlock_start, h.hlock_range, F_RDLCK); - return ntdb->last_error = ecode; + return ecode; } _PUBLIC_ bool ntdb_exists(struct ntdb_context *ntdb, NTDB_DATA key) @@ -273,12 +273,10 @@ _PUBLIC_ bool ntdb_exists(struct ntdb_context *ntdb, NTDB_DATA key) off = find_and_lock(ntdb, key, F_RDLCK, &h, &rec, NULL); if (NTDB_OFF_IS_ERR(off)) { - ntdb->last_error = NTDB_OFF_TO_ERR(off); return false; } ntdb_unlock_hashes(ntdb, h.hlock_start, h.hlock_range, F_RDLCK); - ntdb->last_error = NTDB_SUCCESS; return off ? true : false; } @@ -291,7 +289,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_delete(struct ntdb_context *ntdb, NTDB_DATA key) off = find_and_lock(ntdb, key, F_WRLCK, &h, &rec, NULL); if (NTDB_OFF_IS_ERR(off)) { - return ntdb->last_error = NTDB_OFF_TO_ERR(off); + return NTDB_OFF_TO_ERR(off); } if (!off) { @@ -318,7 +316,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_delete(struct ntdb_context *ntdb, NTDB_DATA key) unlock: ntdb_unlock_hashes(ntdb, h.hlock_start, h.hlock_range, F_WRLCK); - return ntdb->last_error = ecode; + return ecode; } _PUBLIC_ unsigned int ntdb_get_flags(struct ntdb_context *ntdb) @@ -334,11 +332,10 @@ static bool inside_transaction(const struct ntdb_context *ntdb) static bool readonly_changable(struct ntdb_context *ntdb, const char *caller) { if (inside_transaction(ntdb)) { - ntdb->last_error = ntdb_logerr(ntdb, NTDB_ERR_EINVAL, - NTDB_LOG_USE_ERROR, - "%s: can't change" - " NTDB_RDONLY inside transaction", - caller); + ntdb_logerr(ntdb, NTDB_ERR_EINVAL, NTDB_LOG_USE_ERROR, + "%s: can't change" + " NTDB_RDONLY inside transaction", + caller); return false; } return true; @@ -347,9 +344,8 @@ static bool readonly_changable(struct ntdb_context *ntdb, const char *caller) _PUBLIC_ void ntdb_add_flag(struct ntdb_context *ntdb, unsigned flag) { if (ntdb->flags & NTDB_INTERNAL) { - ntdb->last_error = ntdb_logerr(ntdb, NTDB_ERR_EINVAL, - NTDB_LOG_USE_ERROR, - "ntdb_add_flag: internal db"); + ntdb_logerr(ntdb, NTDB_ERR_EINVAL, NTDB_LOG_USE_ERROR, + "ntdb_add_flag: internal db"); return; } switch (flag) { @@ -376,19 +372,16 @@ _PUBLIC_ void ntdb_add_flag(struct ntdb_context *ntdb, unsigned flag) ntdb->flags |= NTDB_RDONLY; break; default: - ntdb->last_error = ntdb_logerr(ntdb, NTDB_ERR_EINVAL, - NTDB_LOG_USE_ERROR, - "ntdb_add_flag: Unknown flag %u", - flag); + ntdb_logerr(ntdb, NTDB_ERR_EINVAL, NTDB_LOG_USE_ERROR, + "ntdb_add_flag: Unknown flag %u", flag); } } _PUBLIC_ void ntdb_remove_flag(struct ntdb_context *ntdb, unsigned flag) { if (ntdb->flags & NTDB_INTERNAL) { - ntdb->last_error = ntdb_logerr(ntdb, NTDB_ERR_EINVAL, - NTDB_LOG_USE_ERROR, - "ntdb_remove_flag: internal db"); + ntdb_logerr(ntdb, NTDB_ERR_EINVAL, NTDB_LOG_USE_ERROR, + "ntdb_remove_flag: internal db"); return; } switch (flag) { @@ -413,21 +406,19 @@ _PUBLIC_ void ntdb_remove_flag(struct ntdb_context *ntdb, unsigned flag) break; case NTDB_RDONLY: if ((ntdb->open_flags & O_ACCMODE) == O_RDONLY) { - ntdb->last_error = ntdb_logerr(ntdb, NTDB_ERR_EINVAL, - NTDB_LOG_USE_ERROR, - "ntdb_remove_flag: can't" - " remove NTDB_RDONLY on ntdb" - " opened with O_RDONLY"); + ntdb_logerr(ntdb, NTDB_ERR_EINVAL, NTDB_LOG_USE_ERROR, + "ntdb_remove_flag: can't" + " remove NTDB_RDONLY on ntdb" + " opened with O_RDONLY"); break; } if (readonly_changable(ntdb, "ntdb_remove_flag")) ntdb->flags &= ~NTDB_RDONLY; break; default: - ntdb->last_error = ntdb_logerr(ntdb, NTDB_ERR_EINVAL, - NTDB_LOG_USE_ERROR, - "ntdb_remove_flag: Unknown flag %u", - flag); + ntdb_logerr(ntdb, NTDB_ERR_EINVAL, NTDB_LOG_USE_ERROR, + "ntdb_remove_flag: Unknown flag %u", + flag); } } @@ -448,11 +439,6 @@ _PUBLIC_ const char *ntdb_errorstr(enum NTDB_ERROR ecode) return "Invalid error code"; } -_PUBLIC_ enum NTDB_ERROR ntdb_error(struct ntdb_context *ntdb) -{ - return ntdb->last_error; -} - enum NTDB_ERROR COLD ntdb_logerr(struct ntdb_context *ntdb, enum NTDB_ERROR ecode, enum ntdb_log_level level, @@ -497,7 +483,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_parse_record_(struct ntdb_context *ntdb, off = find_and_lock(ntdb, key, F_RDLCK, &h, &rec, NULL); if (NTDB_OFF_IS_ERR(off)) { - return ntdb->last_error = NTDB_OFF_TO_ERR(off); + return NTDB_OFF_TO_ERR(off); } if (!off) { @@ -517,7 +503,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_parse_record_(struct ntdb_context *ntdb, } ntdb_unlock_hashes(ntdb, h.hlock_start, h.hlock_range, F_RDLCK); - return ntdb->last_error = ecode; + return ecode; } _PUBLIC_ const char *ntdb_name(const struct ntdb_context *ntdb) @@ -527,14 +513,7 @@ _PUBLIC_ const char *ntdb_name(const struct ntdb_context *ntdb) _PUBLIC_ int64_t ntdb_get_seqnum(struct ntdb_context *ntdb) { - ntdb_off_t off; - - off = ntdb_read_off(ntdb, offsetof(struct ntdb_header, seqnum)); - if (NTDB_OFF_IS_ERR(off)) - ntdb->last_error = NTDB_OFF_TO_ERR(off); - else - ntdb->last_error = NTDB_SUCCESS; - return off; + return ntdb_read_off(ntdb, offsetof(struct ntdb_header, seqnum)); } @@ -577,7 +556,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_repack(struct ntdb_context *ntdb) __location__ " Failed to create tmp_db"); ntdb_transaction_cancel(ntdb); - return ntdb->last_error = state.error; + return state.error; } state.dest_db = tmp_db; diff --git a/lib/ntdb/ntdb.h b/lib/ntdb/ntdb.h index f0833b7261..f64b2f4a1b 100644 --- a/lib/ntdb/ntdb.h +++ b/lib/ntdb/ntdb.h @@ -567,17 +567,6 @@ enum NTDB_ERROR ntdb_check_(struct ntdb_context *ntdb, void *data), void *data); -/** - * ntdb_error - get the last error (not threadsafe) - * @ntdb: the ntdb context returned from ntdb_open() - * - * Returns the last error returned by a NTDB function. - * - * This makes porting from TDB easier, but note that the last error is not - * reliable in threaded programs. - */ -enum NTDB_ERROR ntdb_error(struct ntdb_context *ntdb); - /** * enum ntdb_summary_flags - flags for ntdb_summary. */ diff --git a/lib/ntdb/open.c b/lib/ntdb/open.c index 338de8be8c..28aff0a2ea 100644 --- a/lib/ntdb/open.c +++ b/lib/ntdb/open.c @@ -221,34 +221,31 @@ _PUBLIC_ enum NTDB_ERROR ntdb_set_attribute(struct ntdb_context *ntdb, case NTDB_ATTRIBUTE_HASH: case NTDB_ATTRIBUTE_SEED: case NTDB_ATTRIBUTE_OPENHOOK: - return ntdb->last_error - = ntdb_logerr(ntdb, NTDB_ERR_EINVAL, - NTDB_LOG_USE_ERROR, - "ntdb_set_attribute:" - " cannot set %s after opening", - attr->base.attr == NTDB_ATTRIBUTE_HASH - ? "NTDB_ATTRIBUTE_HASH" - : attr->base.attr == NTDB_ATTRIBUTE_SEED - ? "NTDB_ATTRIBUTE_SEED" - : "NTDB_ATTRIBUTE_OPENHOOK"); + return ntdb_logerr(ntdb, NTDB_ERR_EINVAL, + NTDB_LOG_USE_ERROR, + "ntdb_set_attribute:" + " cannot set %s after opening", + attr->base.attr == NTDB_ATTRIBUTE_HASH + ? "NTDB_ATTRIBUTE_HASH" + : attr->base.attr == NTDB_ATTRIBUTE_SEED + ? "NTDB_ATTRIBUTE_SEED" + : "NTDB_ATTRIBUTE_OPENHOOK"); case NTDB_ATTRIBUTE_STATS: - return ntdb->last_error - = ntdb_logerr(ntdb, NTDB_ERR_EINVAL, - NTDB_LOG_USE_ERROR, - "ntdb_set_attribute:" - " cannot set NTDB_ATTRIBUTE_STATS"); + return ntdb_logerr(ntdb, NTDB_ERR_EINVAL, + NTDB_LOG_USE_ERROR, + "ntdb_set_attribute:" + " cannot set NTDB_ATTRIBUTE_STATS"); case NTDB_ATTRIBUTE_FLOCK: ntdb->lock_fn = attr->flock.lock; ntdb->unlock_fn = attr->flock.unlock; ntdb->lock_data = attr->flock.data; break; default: - return ntdb->last_error - = ntdb_logerr(ntdb, NTDB_ERR_EINVAL, - NTDB_LOG_USE_ERROR, - "ntdb_set_attribute:" - " unknown attribute type %u", - attr->base.attr); + return ntdb_logerr(ntdb, NTDB_ERR_EINVAL, + NTDB_LOG_USE_ERROR, + "ntdb_set_attribute:" + " unknown attribute type %u", + attr->base.attr); } return NTDB_SUCCESS; } @@ -259,7 +256,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_get_attribute(struct ntdb_context *ntdb, switch (attr->base.attr) { case NTDB_ATTRIBUTE_LOG: if (!ntdb->log_fn) - return ntdb->last_error = NTDB_ERR_NOEXIST; + return NTDB_ERR_NOEXIST; attr->log.fn = ntdb->log_fn; attr->log.data = ntdb->log_data; break; @@ -272,7 +269,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_get_attribute(struct ntdb_context *ntdb, break; case NTDB_ATTRIBUTE_OPENHOOK: if (!ntdb->openhook) - return ntdb->last_error = NTDB_ERR_NOEXIST; + return NTDB_ERR_NOEXIST; attr->openhook.fn = ntdb->openhook; attr->openhook.data = ntdb->openhook_data; break; @@ -289,12 +286,11 @@ _PUBLIC_ enum NTDB_ERROR ntdb_get_attribute(struct ntdb_context *ntdb, attr->flock.data = ntdb->lock_data; break; default: - return ntdb->last_error - = ntdb_logerr(ntdb, NTDB_ERR_EINVAL, - NTDB_LOG_USE_ERROR, - "ntdb_get_attribute:" - " unknown attribute type %u", - attr->base.attr); + return ntdb_logerr(ntdb, NTDB_ERR_EINVAL, + NTDB_LOG_USE_ERROR, + "ntdb_get_attribute:" + " unknown attribute type %u", + attr->base.attr); } attr->base.next = NULL; return NTDB_SUCCESS; @@ -415,7 +411,6 @@ _PUBLIC_ struct ntdb_context *ntdb_open(const char *name, int ntdb_flags, ntdb->flags = ntdb_flags; ntdb->log_fn = NULL; ntdb->open_flags = open_flags; - ntdb->last_error = NTDB_SUCCESS; ntdb->file = NULL; ntdb->openhook = NULL; ntdb->lock_fn = ntdb_fcntl_lock; diff --git a/lib/ntdb/private.h b/lib/ntdb/private.h index 1cf9b7aca4..e1c2fdafc4 100644 --- a/lib/ntdb/private.h +++ b/lib/ntdb/private.h @@ -596,9 +596,6 @@ struct ntdb_context { enum NTDB_ERROR (*openhook)(int fd, void *data); void *openhook_data; - /* Last error we returned. */ - enum NTDB_ERROR last_error; - /* Are we accessing directly? (debugging check). */ int direct_access; diff --git a/lib/ntdb/pyntdb.c b/lib/ntdb/pyntdb.c index 1f80e4227b..1037f3c004 100644 --- a/lib/ntdb/pyntdb.c +++ b/lib/ntdb/pyntdb.c @@ -275,8 +275,6 @@ static PyObject *obj_has_key(PyNtdbObject *self, PyObject *args) key = PyString_AsNtdb_Data(py_key); if (ntdb_exists(self->ctx, key)) return Py_True; - if (ntdb_error(self->ctx) != NTDB_ERR_NOEXIST) - PyErr_NTDB_ERROR_IS_ERR_RAISE(ntdb_error(self->ctx)); return Py_False; } diff --git a/lib/ntdb/summary.c b/lib/ntdb/summary.c index 28ffd61df9..571d48ff4d 100644 --- a/lib/ntdb/summary.c +++ b/lib/ntdb/summary.c @@ -209,13 +209,13 @@ _PUBLIC_ enum NTDB_ERROR ntdb_summary(struct ntdb_context *ntdb, ecode = ntdb_allrecord_lock(ntdb, F_RDLCK, NTDB_LOCK_WAIT, false); if (ecode != NTDB_SUCCESS) { - return ntdb->last_error = ecode; + return ecode; } ecode = ntdb_lock_expand(ntdb, F_RDLCK); if (ecode != NTDB_SUCCESS) { ntdb_allrecord_unlock(ntdb, F_RDLCK); - return ntdb->last_error = ecode; + return ecode; } /* Start stats off empty. */ @@ -326,5 +326,5 @@ unlock: ntdb_allrecord_unlock(ntdb, F_RDLCK); ntdb_unlock_expand(ntdb, F_RDLCK); - return ntdb->last_error = ecode; + return ecode; } diff --git a/lib/ntdb/test/failtest_helper.h b/lib/ntdb/test/failtest_helper.h index e754636402..d347d75fba 100644 --- a/lib/ntdb/test/failtest_helper.h +++ b/lib/ntdb/test/failtest_helper.h @@ -4,7 +4,7 @@ #include /* FIXME: Check these! */ -#define INITIAL_NTDB_MALLOC "open.c", 403, FAILTEST_MALLOC +#define INITIAL_NTDB_MALLOC "open.c", 399, FAILTEST_MALLOC #define URANDOM_OPEN "open.c", 62, FAILTEST_OPEN #define URANDOM_READ "open.c", 42, FAILTEST_READ diff --git a/lib/ntdb/tools/ntdbtorture.c b/lib/ntdb/tools/ntdbtorture.c index c7b249db06..d611563a7a 100644 --- a/lib/ntdb/tools/ntdbtorture.c +++ b/lib/ntdb/tools/ntdbtorture.c @@ -80,11 +80,14 @@ static void segv_handler(int sig, siginfo_t *info, void *p) _exit(11); } -static void fatal(struct ntdb_context *ntdb, const char *why) +static void warn_on_err(enum NTDB_ERROR e, struct ntdb_context *ntdb, + const char *why) { - fprintf(stderr, "%u:%s:%s\n", getpid(), why, - ntdb ? ntdb_errorstr(ntdb_error(ntdb)) : "(no ntdb)"); - error_count++; + if (e != NTDB_SUCCESS) { + fprintf(stderr, "%u:%s:%s\n", getpid(), why, + ntdb ? ntdb_errorstr(e) : "(no ntdb)"); + error_count++; + } } static char *randbuf(int len) @@ -129,6 +132,7 @@ static void addrec_db(void) int klen, dlen; char *k, *d; NTDB_DATA key, data; + enum NTDB_ERROR e; klen = 1 + (rand() % KEYLEN); dlen = 1 + (rand() % DATALEN); @@ -151,21 +155,18 @@ static void addrec_db(void) #if TRANSACTION_PROB if (in_traverse == 0 && in_transaction == 0 && (always_transaction || random() % TRANSACTION_PROB == 0)) { - if (ntdb_transaction_start(db) != 0) { - fatal(db, "ntdb_transaction_start failed"); - } + e = ntdb_transaction_start(db); + warn_on_err(e, db, "ntdb_transaction_start failed"); in_transaction++; goto next; } if (in_traverse == 0 && in_transaction && random() % TRANSACTION_PROB == 0) { if (random() % TRANSACTION_PREPARE_PROB == 0) { - if (ntdb_transaction_prepare_commit(db) != 0) { - fatal(db, "ntdb_transaction_prepare_commit failed"); - } - } - if (ntdb_transaction_commit(db) != 0) { - fatal(db, "ntdb_transaction_commit failed"); + e = ntdb_transaction_prepare_commit(db); + warn_on_err(e, db, "ntdb_transaction_prepare_commit failed"); } + e = ntdb_transaction_commit(db); + warn_on_err(e, db, "ntdb_transaction_commit failed"); in_transaction--; goto next; } @@ -186,18 +187,16 @@ static void addrec_db(void) #if STORE_PROB if (random() % STORE_PROB == 0) { - if (ntdb_store(db, key, data, NTDB_REPLACE) != 0) { - fatal(db, "ntdb_store failed"); - } + e = ntdb_store(db, key, data, NTDB_REPLACE); + warn_on_err(e, db, "ntdb_store failed"); goto next; } #endif #if APPEND_PROB if (random() % APPEND_PROB == 0) { - if (ntdb_append(db, key, data) != 0) { - fatal(db, "ntdb_append failed"); - } + e = ntdb_append(db, key, data); + warn_on_err(e, db, "ntdb_append failed"); goto next; } #endif @@ -209,9 +208,8 @@ static void addrec_db(void) data.dsize = 0; data.dptr = NULL; } - if (ntdb_store(db, key, data, NTDB_REPLACE) != 0) { - fatal(db, "ntdb_store failed"); - } + e = ntdb_store(db, key, data, NTDB_REPLACE); + warn_on_err(e, db, "ntdb_store failed"); if (data.dptr) free(data.dptr); ntdb_chainunlock(db, key); goto next; @@ -272,7 +270,9 @@ static int run_child(const char *filename, int i, int seed, unsigned num_loops, db = ntdb_open(filename, ntdb_flags, O_RDWR | O_CREAT, 0600, &log_attr); if (!db) { - fatal(NULL, "db open failed"); + fprintf(stderr, "%u:%s:%s\n", getpid(), filename, + "db open failed"); + exit(1); } #if 0 @@ -295,6 +295,8 @@ static int run_child(const char *filename, int i, int seed, unsigned num_loops, } if (error_count == 0) { + enum NTDB_ERROR e; + ntdb_traverse(db, NULL, NULL); #if TRANSACTION_PROB if (always_transaction) { @@ -302,8 +304,12 @@ static int run_child(const char *filename, int i, int seed, unsigned num_loops, ntdb_transaction_cancel(db); in_transaction--; } - if (ntdb_transaction_start(db) != 0) - fatal(db, "ntdb_transaction_start failed"); + e = ntdb_transaction_start(db); + if (e) { + warn_on_err(e, db, + "ntdb_transaction_start failed"); + exit(1); + } } #endif ntdb_traverse(db, traverse_fn, NULL); @@ -311,8 +317,8 @@ static int run_child(const char *filename, int i, int seed, unsigned num_loops, #if TRANSACTION_PROB if (always_transaction) { - if (ntdb_transaction_commit(db) != 0) - fatal(db, "ntdb_transaction_commit failed"); + e = ntdb_transaction_commit(db); + warn_on_err(e, db, "ntdb_transaction_commit failed"); } #endif } @@ -352,6 +358,7 @@ int main(int argc, char * const *argv) int *done; int ntdb_flags = NTDB_DEFAULT; char *test_ntdb; + enum NTDB_ERROR e; log_attr.base.attr = NTDB_ATTRIBUTE_LOG; log_attr.base.next = &seed_attr; @@ -513,11 +520,13 @@ done: db = ntdb_open(test_ntdb, NTDB_DEFAULT, O_RDWR | O_CREAT, 0600, &log_attr); if (!db) { - fatal(db, "db open failed"); + fprintf(stderr, "%u:%s:%s\n", getpid(), test_ntdb, + "db open failed"); exit(1); } - if (ntdb_check(db, NULL, NULL) != 0) { - fatal(db, "db check failed"); + e = ntdb_check(db, NULL, NULL); + if (e) { + warn_on_err(e, db, "db check failed"); exit(1); } ntdb_close(db); diff --git a/lib/ntdb/transaction.c b/lib/ntdb/transaction.c index 76408c3022..9f953a50e3 100644 --- a/lib/ntdb/transaction.c +++ b/lib/ntdb/transaction.c @@ -524,31 +524,26 @@ _PUBLIC_ enum NTDB_ERROR ntdb_transaction_start(struct ntdb_context *ntdb) ntdb->stats.transactions++; /* some sanity checks */ if (ntdb->flags & NTDB_INTERNAL) { - return ntdb->last_error = ntdb_logerr(ntdb, NTDB_ERR_EINVAL, - NTDB_LOG_USE_ERROR, - "ntdb_transaction_start:" - " cannot start a" - " transaction on an" - " internal ntdb"); + return ntdb_logerr(ntdb, NTDB_ERR_EINVAL, NTDB_LOG_USE_ERROR, + "ntdb_transaction_start:" + " cannot start a transaction on an" + " internal ntdb"); } if (ntdb->flags & NTDB_RDONLY) { - return ntdb->last_error = ntdb_logerr(ntdb, NTDB_ERR_RDONLY, - NTDB_LOG_USE_ERROR, - "ntdb_transaction_start:" - " cannot start a" - " transaction on a " - " read-only ntdb"); + return ntdb_logerr(ntdb, NTDB_ERR_RDONLY, NTDB_LOG_USE_ERROR, + "ntdb_transaction_start:" + " cannot start a transaction on a" + " read-only ntdb"); } /* cope with nested ntdb_transaction_start() calls */ if (ntdb->transaction != NULL) { if (!(ntdb->flags & NTDB_ALLOW_NESTING)) { - return ntdb->last_error - = ntdb_logerr(ntdb, NTDB_ERR_IO, - NTDB_LOG_USE_ERROR, - "ntdb_transaction_start:" - " already inside transaction"); + return ntdb_logerr(ntdb, NTDB_ERR_IO, + NTDB_LOG_USE_ERROR, + "ntdb_transaction_start:" + " already inside transaction"); } ntdb->transaction->nesting++; ntdb->stats.transaction_nest++; @@ -559,21 +554,19 @@ _PUBLIC_ enum NTDB_ERROR ntdb_transaction_start(struct ntdb_context *ntdb) /* the caller must not have any locks when starting a transaction as otherwise we'll be screwed by lack of nested locks in POSIX */ - return ntdb->last_error = ntdb_logerr(ntdb, NTDB_ERR_LOCK, - NTDB_LOG_USE_ERROR, - "ntdb_transaction_start:" - " cannot start a" - " transaction with locks" - " held"); + return ntdb_logerr(ntdb, NTDB_ERR_LOCK, + NTDB_LOG_USE_ERROR, + "ntdb_transaction_start:" + " cannot start a transaction with locks" + " held"); } ntdb->transaction = (struct ntdb_transaction *) calloc(sizeof(struct ntdb_transaction), 1); if (ntdb->transaction == NULL) { - return ntdb->last_error = ntdb_logerr(ntdb, NTDB_ERR_OOM, - NTDB_LOG_ERROR, - "ntdb_transaction_start:" - " cannot allocate"); + return ntdb_logerr(ntdb, NTDB_ERR_OOM, NTDB_LOG_ERROR, + "ntdb_transaction_start:" + " cannot allocate"); } /* get the transaction write lock. This is a blocking lock. As @@ -583,7 +576,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_transaction_start(struct ntdb_context *ntdb) if (ecode != NTDB_SUCCESS) { SAFE_FREE(ntdb->transaction->blocks); SAFE_FREE(ntdb->transaction); - return ntdb->last_error = ecode; + return ecode; } /* get a read lock over entire file. This is upgraded to a write @@ -602,13 +595,13 @@ _PUBLIC_ enum NTDB_ERROR ntdb_transaction_start(struct ntdb_context *ntdb) transaction specific methods */ ntdb->transaction->io_methods = ntdb->io; ntdb->io = &transaction_methods; - return ntdb->last_error = NTDB_SUCCESS; + return NTDB_SUCCESS; fail_allrecord_lock: ntdb_transaction_unlock(ntdb, F_WRLCK); SAFE_FREE(ntdb->transaction->blocks); SAFE_FREE(ntdb->transaction); - return ntdb->last_error = ecode; + return ecode; } @@ -1057,7 +1050,7 @@ static enum NTDB_ERROR _ntdb_transaction_prepare_commit(struct ntdb_context *ntd */ _PUBLIC_ enum NTDB_ERROR ntdb_transaction_prepare_commit(struct ntdb_context *ntdb) { - return ntdb->last_error = _ntdb_transaction_prepare_commit(ntdb); + return _ntdb_transaction_prepare_commit(ntdb); } /* @@ -1070,30 +1063,29 @@ _PUBLIC_ enum NTDB_ERROR ntdb_transaction_commit(struct ntdb_context *ntdb) enum NTDB_ERROR ecode; if (ntdb->transaction == NULL) { - return ntdb->last_error = ntdb_logerr(ntdb, NTDB_ERR_EINVAL, - NTDB_LOG_USE_ERROR, - "ntdb_transaction_commit:" - " no transaction"); + return ntdb_logerr(ntdb, NTDB_ERR_EINVAL, NTDB_LOG_USE_ERROR, + "ntdb_transaction_commit:" + " no transaction"); } ntdb_trace(ntdb, "ntdb_transaction_commit"); if (ntdb->transaction->nesting != 0) { ntdb->transaction->nesting--; - return ntdb->last_error = NTDB_SUCCESS; + return NTDB_SUCCESS; } /* check for a null transaction */ if (ntdb->transaction->blocks == NULL) { _ntdb_transaction_cancel(ntdb); - return ntdb->last_error = NTDB_SUCCESS; + return NTDB_SUCCESS; } if (!ntdb->transaction->prepared) { ecode = _ntdb_transaction_prepare_commit(ntdb); if (ecode != NTDB_SUCCESS) { _ntdb_transaction_cancel(ntdb); - return ntdb->last_error = ecode; + return ecode; } } @@ -1125,7 +1117,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_transaction_commit(struct ntdb_context *ntdb) _ntdb_transaction_cancel(ntdb); - return ntdb->last_error = ecode; + return ecode; } SAFE_FREE(ntdb->transaction->blocks[i]); } @@ -1136,7 +1128,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_transaction_commit(struct ntdb_context *ntdb) /* ensure the new data is on disk */ ecode = transaction_sync(ntdb, 0, ntdb->file->map_size); if (ecode != NTDB_SUCCESS) { - return ntdb->last_error = ecode; + return ecode; } /* @@ -1159,7 +1151,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_transaction_commit(struct ntdb_context *ntdb) ntdb->transaction->old_map_size = ntdb->file->map_size; _ntdb_transaction_cancel(ntdb); - return ntdb->last_error = NTDB_SUCCESS; + return NTDB_SUCCESS; } diff --git a/lib/ntdb/traverse.c b/lib/ntdb/traverse.c index 52bf75c684..45478755bd 100644 --- a/lib/ntdb/traverse.c +++ b/lib/ntdb/traverse.c @@ -37,16 +37,14 @@ _PUBLIC_ int64_t ntdb_traverse_(struct ntdb_context *ntdb, count++; if (fn && fn(ntdb, k, d, p)) { free(k.dptr); - ntdb->last_error = NTDB_SUCCESS; return count; } free(k.dptr); } if (ecode != NTDB_ERR_NOEXIST) { - return NTDB_ERR_TO_OFF(ntdb->last_error = ecode); + return NTDB_ERR_TO_OFF(ecode); } - ntdb->last_error = NTDB_SUCCESS; return count; } @@ -54,7 +52,7 @@ _PUBLIC_ enum NTDB_ERROR ntdb_firstkey(struct ntdb_context *ntdb, NTDB_DATA *key { struct traverse_info tinfo; - return ntdb->last_error = first_in_hash(ntdb, &tinfo, key, NULL); + return first_in_hash(ntdb, &tinfo, key, NULL); } /* We lock twice, not very efficient. We could keep last key & tinfo cached. */ @@ -67,11 +65,11 @@ _PUBLIC_ enum NTDB_ERROR ntdb_nextkey(struct ntdb_context *ntdb, NTDB_DATA *key) tinfo.prev = find_and_lock(ntdb, *key, F_RDLCK, &h, &rec, &tinfo); free(key->dptr); if (NTDB_OFF_IS_ERR(tinfo.prev)) { - return ntdb->last_error = NTDB_OFF_TO_ERR(tinfo.prev); + return NTDB_OFF_TO_ERR(tinfo.prev); } ntdb_unlock_hashes(ntdb, h.hlock_start, h.hlock_range, F_RDLCK); - return ntdb->last_error = next_in_hash(ntdb, &tinfo, key, NULL); + return next_in_hash(ntdb, &tinfo, key, NULL); } static int wipe_one(struct ntdb_context *ntdb, @@ -88,12 +86,12 @@ _PUBLIC_ enum NTDB_ERROR ntdb_wipe_all(struct ntdb_context *ntdb) ecode = ntdb_allrecord_lock(ntdb, F_WRLCK, NTDB_LOCK_WAIT, false); if (ecode != NTDB_SUCCESS) - return ntdb->last_error = ecode; + return ecode; /* FIXME: Be smarter. */ count = ntdb_traverse(ntdb, wipe_one, &ecode); if (count < 0) ecode = NTDB_OFF_TO_ERR(count); ntdb_allrecord_unlock(ntdb, F_WRLCK); - return ntdb->last_error = ecode; + return ecode; } -- cgit