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

@@ -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]);
  *	

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;
 }

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;

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);
 }

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;
 }

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.

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;

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
 

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);
 		}

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);

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);
 		}

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);

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);

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);

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);

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. */

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 *)&num;
 		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 *)&num;
 		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 *)&num;
@@ -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",

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);

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"));

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();
 }

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;

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;
 }
 
 

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);
 }