dbwrap: Pass "value" to dbwrap_do_locked() callback

I want to reduce dbwrap_record_get_value(). It makes the caller believe it can
make a copy of the TDB_DATA returned and that the value remains constant. It's
not, as you can always do a dbwrap_record_store().

This patch removes one requirement for getting the value out of a
db_record via dbwrap_record_get_value(). You can still make a copy, but from an
API perspective to me it's more obvious that "value" as a parameter to the
callback has a limited lifetime.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

10 files changed, 71 insertions, 38 deletions

diff --git a/lib/dbwrap/dbwrap.c b/lib/dbwrap/dbwrap.c
index 3d8d2ae78d7..cc58397eb6e 100644
--- a/lib/dbwrap/dbwrap.c
+++ b/lib/dbwrap/dbwrap.c
@@ -307,7 +307,10 @@ struct dbwrap_store_state {
 	NTSTATUS status;
 };
 
-static void dbwrap_store_fn(struct db_record *rec, void *private_data)
+static void dbwrap_store_fn(
+	struct db_record *rec,
+	TDB_DATA value,
+	void *private_data)
 {
 	struct dbwrap_store_state *state = private_data;
 	state->status = dbwrap_record_store(rec, state->data, state->flags);
@@ -331,7 +334,10 @@ struct dbwrap_delete_state {
 	NTSTATUS status;
 };
 
-static void dbwrap_delete_fn(struct db_record *rec, void *private_data)
+static void dbwrap_delete_fn(
+	struct db_record *rec,
+	TDB_DATA value,
+	void *private_data)
 {
 	struct dbwrap_delete_state *state = private_data;
 	state->status = dbwrap_record_delete(rec);
@@ -514,6 +520,7 @@ NTSTATUS dbwrap_parse_record_recv(struct tevent_req *req)
 
 NTSTATUS dbwrap_do_locked(struct db_context *db, TDB_DATA key,
 			  void (*fn)(struct db_record *rec,
+				     TDB_DATA value,
 				     void *private_data),
 			  void *private_data)
 {
@@ -542,7 +549,7 @@ NTSTATUS dbwrap_do_locked(struct db_context *db, TDB_DATA key,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	fn(rec, private_data);
+	fn(rec, rec->value, private_data);
 
 	TALLOC_FREE(rec);
 
diff --git a/lib/dbwrap/dbwrap.h b/lib/dbwrap/dbwrap.h
index 591eac5c63a..b5a7fb315c7 100644
--- a/lib/dbwrap/dbwrap.h
+++ b/lib/dbwrap/dbwrap.h
@@ -81,6 +81,7 @@ struct db_context *dbwrap_record_get_db(struct db_record *rec);
 
 NTSTATUS dbwrap_do_locked(struct db_context *db, TDB_DATA key,
 			  void (*fn)(struct db_record *rec,
+				     TDB_DATA value,
 				     void *private_data),
 			  void *private_data);
 
diff --git a/lib/dbwrap/dbwrap_private.h b/lib/dbwrap/dbwrap_private.h
index b548168b9a9..a493b261dcb 100644
--- a/lib/dbwrap/dbwrap_private.h
+++ b/lib/dbwrap/dbwrap_private.h
@@ -70,6 +70,7 @@ struct db_context {
 	NTSTATUS (*parse_record_recv)(struct tevent_req *req);
 	NTSTATUS (*do_locked)(struct db_context *db, TDB_DATA key,
 			      void (*fn)(struct db_record *rec,
+					 TDB_DATA value,
 					 void *private_data),
 			      void *private_data);
 	int (*exists)(struct db_context *db,TDB_DATA key);
diff --git a/lib/dbwrap/dbwrap_tdb.c b/lib/dbwrap/dbwrap_tdb.c
index ed20f885278..b7bba40705a 100644
--- a/lib/dbwrap/dbwrap_tdb.c
+++ b/lib/dbwrap/dbwrap_tdb.c
@@ -184,6 +184,7 @@ static struct db_record *db_tdb_try_fetch_locked(
 
 static NTSTATUS db_tdb_do_locked(struct db_context *db, TDB_DATA key,
 				 void (*fn)(struct db_record *rec,
+					    TDB_DATA value,
 					    void *private_data),
 				 void *private_data)
 {
@@ -218,7 +219,7 @@ static NTSTATUS db_tdb_do_locked(struct db_context *db, TDB_DATA key,
 		.private_data = ctx
 	};
 
-	fn(&rec, private_data);
+	fn(&rec, rec.value, private_data);
 
 	tdb_chainunlock(ctx->wtdb->tdb, key);
 
diff --git a/source3/lib/dbwrap/dbwrap_watch.c b/source3/lib/dbwrap/dbwrap_watch.c
index c5d55a3c93d..41cefbd1533 100644
--- a/source3/lib/dbwrap/dbwrap_watch.c
+++ b/source3/lib/dbwrap/dbwrap_watch.c
@@ -279,7 +279,9 @@ static struct db_record *dbwrap_watched_fetch_locked(
 struct dbwrap_watched_do_locked_state {
 	TALLOC_CTX *mem_ctx;
 	struct db_context *db;
-	void (*fn)(struct db_record *rec, void *private_data);
+	void (*fn)(struct db_record *rec,
+		   TDB_DATA value,
+		   void *private_data);
 	void *private_data;
 
 	struct db_watched_subrec subrec;
@@ -310,17 +312,19 @@ static NTSTATUS dbwrap_watched_do_locked_delete(struct db_record *rec)
 	return status;
 }
 
-static void dbwrap_watched_do_locked_fn(struct db_record *subrec,
-					void *private_data)
+static void dbwrap_watched_do_locked_fn(
+	struct db_record *subrec,
+	TDB_DATA subrec_value,
+	void *private_data)
 {
 	struct dbwrap_watched_do_locked_state *state =
 		(struct dbwrap_watched_do_locked_state *)private_data;
-	TDB_DATA subrec_value = dbwrap_record_get_value(subrec);
 	struct db_record rec;
 	bool ok;
 
 	rec = (struct db_record) {
-		.db = state->db, .key = dbwrap_record_get_key(subrec),
+		.db = state->db,
+		.key = dbwrap_record_get_key(subrec),
 		.storev = dbwrap_watched_do_locked_storev,
 		.delete_rec = dbwrap_watched_do_locked_delete,
 		.private_data = state
@@ -349,11 +353,12 @@ static void dbwrap_watched_do_locked_fn(struct db_record *subrec,
 		rec.value = state->subrec.wrec.data;
 	}
 
-	state->fn(&rec, state->private_data);
+	state->fn(&rec, state->subrec.wrec.data, state->private_data);
 }
 
 static NTSTATUS dbwrap_watched_do_locked(struct db_context *db, TDB_DATA key,
 					 void (*fn)(struct db_record *rec,
+						    TDB_DATA value,
 						    void *private_data),
 					 void *private_data)
 {
diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index b01bb591f77..8c67a599173 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -348,7 +348,10 @@ struct g_lock_lock_fn_state {
 	NTSTATUS status;
 };
 
-static void g_lock_lock_fn(struct db_record *rec, void *private_data)
+static void g_lock_lock_fn(
+	struct db_record *rec,
+	TDB_DATA value,
+	void *private_data)
 {
 	struct g_lock_lock_fn_state *state = private_data;
 	struct server_id blocker = {0};
@@ -509,17 +512,16 @@ struct g_lock_unlock_state {
 	NTSTATUS status;
 };
 
-static void g_lock_unlock_fn(struct db_record *rec,
-			     void *private_data)
+static void g_lock_unlock_fn(
+	struct db_record *rec,
+	TDB_DATA value,
+	void *private_data)
 {
 	struct g_lock_unlock_state *state = private_data;
-	TDB_DATA value;
 	struct g_lock lck;
 	size_t i;
 	bool ok;
 
-	value = dbwrap_record_get_value(rec);
-
 	ok = g_lock_parse(value.dptr, value.dsize, &lck);
 	if (!ok) {
 		DBG_DEBUG("g_lock_parse for %s failed\n",
@@ -581,17 +583,16 @@ struct g_lock_write_data_state {
 	NTSTATUS status;
 };
 
-static void g_lock_write_data_fn(struct db_record *rec,
-				 void *private_data)
+static void g_lock_write_data_fn(
+	struct db_record *rec,
+	TDB_DATA value,
+	void *private_data)
 {
 	struct g_lock_write_data_state *state = private_data;
-	TDB_DATA value;
 	struct g_lock lck;
 	size_t i;
 	bool ok;
 
-	value = dbwrap_record_get_value(rec);
-
 	ok = g_lock_parse(value.dptr, value.dsize, &lck);
 	if (!ok) {
 		DBG_DEBUG("g_lock_parse for %s failed\n",
diff --git a/source3/locking/leases_db.c b/source3/locking/leases_db.c
index 19c5a612ecc..a12b421d260 100644
--- a/source3/locking/leases_db.c
+++ b/source3/locking/leases_db.c
@@ -97,10 +97,12 @@ struct leases_db_do_locked_state {
 	NTSTATUS status;
 };
 
-static void leases_db_do_locked_fn(struct db_record *rec, void *private_data)
+static void leases_db_do_locked_fn(
+	struct db_record *rec,
+	TDB_DATA db_value,
+	void *private_data)
 {
 	struct leases_db_do_locked_state *state = private_data;
-	TDB_DATA db_value = dbwrap_record_get_value(rec);
 	DATA_BLOB blob = { .data = db_value.dptr, .length = db_value.dsize };
 	struct leases_db_value *value = NULL;
 	enum ndr_err_code ndr_err;
diff --git a/source3/locking/posix.c b/source3/locking/posix.c
index 91485133eb2..e74f48636ec 100644
--- a/source3/locking/posix.c
+++ b/source3/locking/posix.c
@@ -513,11 +513,13 @@ struct add_fd_to_close_entry_state {
 };
 
 static void add_fd_to_close_entry_fn(
-	struct db_record *rec, void *private_data)
+	struct db_record *rec,
+	TDB_DATA value,
+	void *private_data)
 {
 	struct add_fd_to_close_entry_state *state = private_data;
 	TDB_DATA values[] = {
-		dbwrap_record_get_value(rec),
+		value,
 		{ .dptr = (uint8_t *)&(state->fsp->fh->fd),
 		  .dsize = sizeof(state->fsp->fh->fd) },
 	};
@@ -551,9 +553,10 @@ static void add_fd_to_close_entry(const files_struct *fsp)
 }
 
 static void fd_close_posix_fn(
-	struct db_record *rec, void *private_data)
+	struct db_record *rec,
+	TDB_DATA data,
+	void *private_data)
 {
-	TDB_DATA data = dbwrap_record_get_value(rec);
 	size_t num_fds, i;
 
 	SMB_ASSERT((data.dsize % sizeof(int)) == 0);
diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index 234dd90c1d0..a4e52d6a5b0 100644
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -720,13 +720,14 @@ struct share_mode_do_locked_state {
 	void *private_data;
 };
 
-static void share_mode_do_locked_fn(struct db_record *rec,
-				    void *private_data)
+static void share_mode_do_locked_fn(
+	struct db_record *rec,
+	TDB_DATA value,
+	void *private_data)
 {
 	struct share_mode_do_locked_state *state = private_data;
 	bool modified_dependent = false;
 	bool reset_static_share_mode_record = false;
-	TDB_DATA value = dbwrap_record_get_value(rec);
 
 	if (static_share_mode_record == NULL) {
 		static_share_mode_record = rec;
@@ -1415,10 +1416,12 @@ struct set_share_mode_state {
 	NTSTATUS status;
 };
 
-static void set_share_mode_fn(struct db_record *rec, void *private_data)
+static void set_share_mode_fn(
+	struct db_record *rec,
+	TDB_DATA data,
+	void *private_data)
 {
 	struct set_share_mode_state *state = private_data;
-	TDB_DATA data = dbwrap_record_get_value(rec);
 	size_t idx, num_share_modes;
 	struct share_mode_entry tmp;
 	struct share_mode_entry_buf buf;
@@ -1651,11 +1654,12 @@ static bool share_mode_for_one_entry(
 }
 
 static void share_mode_forall_entries_fn(
-	struct db_record *rec, void *private_data)
+	struct db_record *rec,
+	TDB_DATA data,
+	void *private_data)
 {
 	struct share_mode_forall_entries_state *state = private_data;
 	struct share_mode_data *d = state->lck->data;
-	struct TDB_DATA data = dbwrap_record_get_value(rec);
 	size_t num_share_modes;
 	bool writeback = false;
 	NTSTATUS status;
@@ -1757,10 +1761,12 @@ struct share_mode_entry_do_state {
 	NTSTATUS status;
 };
 
-static void share_mode_entry_do_fn(struct db_record *rec, void *private_data)
+static void share_mode_entry_do_fn(
+	struct db_record *rec,
+	TDB_DATA data,
+	void *private_data)
 {
 	struct share_mode_entry_do_state *state = private_data;
-	struct TDB_DATA data = dbwrap_record_get_value(rec);
 	size_t idx;
 	bool found = false;
 	bool modified;
diff --git a/source3/torture/test_dbwrap_do_locked.c b/source3/torture/test_dbwrap_do_locked.c
index a5b9d4400cf..7226ec14128 100644
--- a/source3/torture/test_dbwrap_do_locked.c
+++ b/source3/torture/test_dbwrap_do_locked.c
@@ -31,7 +31,10 @@ struct do_locked1_state {
 	NTSTATUS status;
 };
 
-static void do_locked1_cb(struct db_record *rec, void *private_data)
+static void do_locked1_cb(
+	struct db_record *rec,
+	TDB_DATA value,
+	void *private_data)
 {
 	struct do_locked1_state *state =
 		(struct do_locked1_state *)private_data;
@@ -55,7 +58,10 @@ static void do_locked1_check(TDB_DATA key, TDB_DATA value,
 	state->status = NT_STATUS_OK;
 }
 
-static void do_locked1_del(struct db_record *rec, void *private_data)
+static void do_locked1_del(
+	struct db_record *rec,
+	TDB_DATA value,
+	void *private_data)
 {
 	struct do_locked1_state *state =
 		(struct do_locked1_state *)private_data;