s3:dbwrap: turn the fetch dbwrap method to NTSTATUS return code.

This implement more correct NTSTATUS handling inside the backends.
This ensures that data.dptr != NULL if return code is NT_STATUS_OK.

5 files changed, 58 insertions, 55 deletions

diff --git a/source3/lib/dbwrap/dbwrap.c b/source3/lib/dbwrap/dbwrap.c
index 38404a876c..cdc46c328f 100644
--- a/source3/lib/dbwrap/dbwrap.c
+++ b/source3/lib/dbwrap/dbwrap.c
@@ -28,19 +28,20 @@
  * Fall back using fetch_locked if no genuine fetch operation is provided
  */
 
-int dbwrap_fallback_fetch(struct db_context *db, TALLOC_CTX *mem_ctx,
-			  TDB_DATA key, TDB_DATA *data)
+NTSTATUS dbwrap_fallback_fetch(struct db_context *db, TALLOC_CTX *mem_ctx,
+			       TDB_DATA key, TDB_DATA *data)
 {
 	struct db_record *rec;
 
-	if (!(rec = db->fetch_locked(db, mem_ctx, key))) {
-		return -1;
+	rec = db->fetch_locked(db, mem_ctx, key);
+	if (rec == NULL) {
+		return NT_STATUS_UNSUCCESSFUL;
 	}
 
 	data->dsize = rec->value.dsize;
 	data->dptr = talloc_move(mem_ctx, &rec->value.dptr);
 	TALLOC_FREE(rec);
-	return 0;
+	return NT_STATUS_OK;
 }
 
 /*
@@ -65,9 +66,10 @@ int dbwrap_fallback_parse_record(struct db_context *db, TDB_DATA key,
 {
 	TDB_DATA data;
 	int res;
+	NTSTATUS status;
 
-	res = db->fetch(db, talloc_tos(), key, &data);
-	if (res != 0) {
+	status = db->fetch(db, talloc_tos(), key, &data);
+	if (!NT_STATUS_IS_OK(status)) {
 		return -1;
 	}
 
@@ -137,11 +139,7 @@ NTSTATUS dbwrap_fetch(struct db_context *db, TALLOC_CTX *mem_ctx,
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
-	if (db->fetch(db, mem_ctx, key, value) != 0) {
-		return NT_STATUS_NOT_FOUND;
-	}
-
-	return NT_STATUS_OK;
+	return db->fetch(db, mem_ctx, key, value);
 }
 
 bool dbwrap_exists(struct db_context *db, TDB_DATA key)
diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index 234aeebae3..7262b873fa 100644
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -456,9 +456,9 @@ static bool pull_newest_from_marshall_buffer(struct ctdb_marshall_buffer *buf,
 /*
   fetch a record inside a transaction
  */
-static int db_ctdb_transaction_fetch(struct db_ctdb_ctx *db, 
-				     TALLOC_CTX *mem_ctx, 
-				     TDB_DATA key, TDB_DATA *data)
+static NTSTATUS db_ctdb_transaction_fetch(struct db_ctdb_ctx *db,
+					  TALLOC_CTX *mem_ctx,
+					  TDB_DATA key, TDB_DATA *data)
 {
 	struct db_ctdb_transaction_handle *h = db->transaction;
 	NTSTATUS status;
@@ -467,18 +467,16 @@ static int db_ctdb_transaction_fetch(struct db_ctdb_ctx *db,
 	found = pull_newest_from_marshall_buffer(h->m_write, key, NULL,
 						 mem_ctx, data);
 	if (found) {
-		return 0;
+		return NT_STATUS_OK;
 	}
 
 	status = db_ctdb_ltdb_fetch(h->ctx, key, NULL, mem_ctx, data);
 
 	if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
 		*data = tdb_null;
-	} else if (!NT_STATUS_IS_OK(status)) {
-		return -1;
 	}
 
-	return 0;
+	return status;
 }
 
 /**
@@ -492,9 +490,9 @@ static int db_ctdb_transaction_fetch(struct db_ctdb_ctx *db,
  * of records bump their RSN and hence render the persistent
  * database inconsistent.
  */
-static int db_ctdb_fetch_persistent(struct db_ctdb_ctx *db,
-				    TALLOC_CTX *mem_ctx,
-				    TDB_DATA key, TDB_DATA *data)
+static NTSTATUS db_ctdb_fetch_persistent(struct db_ctdb_ctx *db,
+					 TALLOC_CTX *mem_ctx,
+					 TDB_DATA key, TDB_DATA *data)
 {
 	NTSTATUS status;
 
@@ -502,11 +500,9 @@ static int db_ctdb_fetch_persistent(struct db_ctdb_ctx *db,
 
 	if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
 		*data = tdb_null;
-	} else if (!NT_STATUS_IS_OK(status)) {
-		return -1;
 	}
 
-	return 0;
+	return status;
 }
 
 static NTSTATUS db_ctdb_store_transaction(struct db_record *rec, TDB_DATA data, int flag);
@@ -1144,8 +1140,8 @@ static struct db_record *db_ctdb_fetch_locked(struct db_context *db,
 /*
   fetch (unlocked, no migration) operation on ctdb
  */
-static int db_ctdb_fetch(struct db_context *db, TALLOC_CTX *mem_ctx,
-			 TDB_DATA key, TDB_DATA *data)
+static NTSTATUS db_ctdb_fetch(struct db_context *db, TALLOC_CTX *mem_ctx,
+			      TDB_DATA key, TDB_DATA *data)
 {
 	struct db_ctdb_ctx *ctx = talloc_get_type_abort(db->private_data,
 							struct db_ctdb_ctx);
@@ -1175,11 +1171,6 @@ static int db_ctdb_fetch(struct db_context *db, TALLOC_CTX *mem_ctx,
 		/* we are the dmaster - avoid the ctdb protocol op */
 
 		data->dsize = ctdb_data.dsize - sizeof(struct ctdb_ltdb_header);
-		if (data->dsize == 0) {
-			SAFE_FREE(ctdb_data.dptr);
-			data->dptr = NULL;
-			return 0;
-		}
 
 		data->dptr = (uint8 *)talloc_memdup(
 			mem_ctx, ctdb_data.dptr+sizeof(struct ctdb_ltdb_header),
@@ -1188,9 +1179,9 @@ static int db_ctdb_fetch(struct db_context *db, TALLOC_CTX *mem_ctx,
 		SAFE_FREE(ctdb_data.dptr);
 
 		if (data->dptr == NULL) {
-			return -1;
+			return NT_STATUS_NO_MEMORY;
 		}
-		return 0;
+		return NT_STATUS_OK;
 	}
 
 	SAFE_FREE(ctdb_data.dptr);
@@ -1200,10 +1191,9 @@ static int db_ctdb_fetch(struct db_context *db, TALLOC_CTX *mem_ctx,
 			     mem_ctx, data);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(5, ("ctdbd_fetch failed: %s\n", nt_errstr(status)));
-		return -1;
 	}
 
-	return 0;
+	return status;
 }
 
 struct traverse_state {
diff --git a/source3/lib/dbwrap/dbwrap_private.h b/source3/lib/dbwrap/dbwrap_private.h
index 0a8581a91b..e7bd480cbb 100644
--- a/source3/lib/dbwrap/dbwrap_private.h
+++ b/source3/lib/dbwrap/dbwrap_private.h
@@ -34,8 +34,8 @@ struct db_context {
 	struct db_record *(*fetch_locked)(struct db_context *db,
 					  TALLOC_CTX *mem_ctx,
 					  TDB_DATA key);
-	int (*fetch)(struct db_context *db, TALLOC_CTX *mem_ctx,
-		     TDB_DATA key, TDB_DATA *data);
+	NTSTATUS (*fetch)(struct db_context *db, TALLOC_CTX *mem_ctx,
+			  TDB_DATA key, TDB_DATA *data);
 	int (*traverse)(struct db_context *db,
 			int (*f)(struct db_record *rec,
 				 void *private_data),
@@ -59,8 +59,8 @@ struct db_context {
 	bool persistent;
 };
 
-int dbwrap_fallback_fetch(struct db_context *db, TALLOC_CTX *mem_ctx,
-			  TDB_DATA key, TDB_DATA *data);
+NTSTATUS dbwrap_fallback_fetch(struct db_context *db, TALLOC_CTX *mem_ctx,
+			       TDB_DATA key, TDB_DATA *data);
 
 
 int dbwrap_fallback_parse_record(struct db_context *db, TDB_DATA key,
diff --git a/source3/lib/dbwrap/dbwrap_rbt.c b/source3/lib/dbwrap/dbwrap_rbt.c
index 98541eaf07..3f280c27de 100644
--- a/source3/lib/dbwrap/dbwrap_rbt.c
+++ b/source3/lib/dbwrap/dbwrap_rbt.c
@@ -344,8 +344,8 @@ static int db_rbt_parse_record(struct db_context *db, TDB_DATA key,
 	return parser(res.key, res.val, private_data);
 }
 
-static int db_rbt_fetch(struct db_context *db, TALLOC_CTX *mem_ctx,
-			TDB_DATA key, TDB_DATA *data)
+static NTSTATUS db_rbt_fetch(struct db_context *db, TALLOC_CTX *mem_ctx,
+			     TDB_DATA key, TDB_DATA *data)
 {
 	uint8_t *result;
 	struct db_rbt_search_result res;
@@ -354,17 +354,17 @@ static int db_rbt_fetch(struct db_context *db, TALLOC_CTX *mem_ctx,
 
 	if (!found) {
 		*data = tdb_null;
-		return 0;
+		return NT_STATUS_NOT_FOUND;
 	}
 
 	result = (uint8_t*)talloc_memdup(mem_ctx, res.val.dptr, res.val.dsize);
 	if (result == NULL) {
-		return -1;
+		return NT_STATUS_NO_MEMORY;
 	}
 
 	data->dptr = result;
 	data->dsize = res.val.dsize;
-	return 0;
+	return NT_STATUS_OK;
 }
 
 static int db_rbt_traverse_internal(struct rb_node *n,
diff --git a/source3/lib/dbwrap/dbwrap_tdb.c b/source3/lib/dbwrap/dbwrap_tdb.c
index e9e4900068..cf761e24f0 100644
--- a/source3/lib/dbwrap/dbwrap_tdb.c
+++ b/source3/lib/dbwrap/dbwrap_tdb.c
@@ -23,6 +23,7 @@
 #include "dbwrap/dbwrap_tdb.h"
 #include "lib/util/tdb_wrap.h"
 #include "lib/param/param.h"
+#include "util_tdb.h"
 
 struct db_tdb_ctx {
 	struct tdb_wrap *wtdb;
@@ -135,7 +136,7 @@ static struct db_record *db_tdb_fetch_locked(struct db_context *db,
 
 struct tdb_fetch_state {
 	TALLOC_CTX *mem_ctx;
-	int result;
+	NTSTATUS result;
 	TDB_DATA data;
 };
 
@@ -145,19 +146,25 @@ static int db_tdb_fetch_parse(TDB_DATA key, TDB_DATA data,
 	struct tdb_fetch_state *state =
 		(struct tdb_fetch_state *)private_data;
 
+	if (data.dptr == NULL) {
+		/* should not happen */
+		state->result = NT_STATUS_INTERNAL_DB_ERROR;
+		return -1;
+	}
+
 	state->data.dptr = (uint8 *)talloc_memdup(state->mem_ctx, data.dptr,
 						  data.dsize);
 	if (state->data.dptr == NULL) {
-		state->result = -1;
-		return 0;
+		state->result = NT_STATUS_NO_MEMORY;
+		return -1;
 	}
 
 	state->data.dsize = data.dsize;
 	return 0;
 }
 
-static int db_tdb_fetch(struct db_context *db, TALLOC_CTX *mem_ctx,
-			TDB_DATA key, TDB_DATA *pdata)
+static NTSTATUS db_tdb_fetch(struct db_context *db, TALLOC_CTX *mem_ctx,
+			     TDB_DATA key, TDB_DATA *pdata)
 {
 	struct db_tdb_ctx *ctx = talloc_get_type_abort(
 		db->private_data, struct db_tdb_ctx);
@@ -166,21 +173,29 @@ static int db_tdb_fetch(struct db_context *db, TALLOC_CTX *mem_ctx,
 	int ret;
 
 	state.mem_ctx = mem_ctx;
-	state.result = 0;
+	state.result = NT_STATUS_OK;
 	state.data = tdb_null;
 
 	ret = tdb_parse_record(ctx->wtdb->tdb, key, db_tdb_fetch_parse, &state);
 
 	if (ret != 0) {
-		return -1;
+		NTSTATUS status;
+
+		if (!NT_STATUS_IS_OK(state.result)) {
+			/* the parser has set an error code. return it */
+			return state.result;
+		}
+
+		status = map_nt_error_from_tdb(tdb_error(ctx->wtdb->tdb));
+		return status;
 	}
 
-	if (state.result == -1) {
-		return -1;
+	if (!NT_STATUS_IS_OK(state.result)) {
+		return NT_STATUS_INTERNAL_DB_CORRUPTION;
 	}
 
 	*pdata = state.data;
-	return 0;
+	return NT_STATUS_OK;
 }
 
 static int db_tdb_exists(struct db_context *db, TDB_DATA key)