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>

diff --git a/lib/dbwrap/dbwrap.c b/lib/dbwrap/dbwrap.c
index 3d8d2ae78d7b8d194fb565e5840a7192f28ed22e..cc58397eb6e35c18180aed11ddf3fe5f378ec6a9 100644 (file)
--- 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 591eac5c63aaa1d07295b8c3fea7995b352ee349..b5a7fb315c77f56f9d63cb4966a64f007d80afe3 100644 (file)
--- 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 b548168b9a961cae9610c61bab7e340391dc3a0a..a493b261dcbf5905cfd243643548626a5d73ffda 100644 (file)
--- 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 ed20f88527867fbb9648806d64ce7956e8a7c338..b7bba40705a7ca417e2c462c259b6e9484f06952 100644 (file)
--- 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 c5d55a3c93dc2d78a848d7fbff6f43495b156e5a..41cefbd153385dfa2da8ca546f5adccc4a92d40a 100644 (file)
--- 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 b01bb591f77102f74d0ae9f0aee4e8766f9271a3..8c67a599173b0d8772d9b662b55924170661e77d 100644 (file)
--- 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 19c5a612ecc71a1da842c56497a6713b033c4d41..a12b421d26034f5ad9461232d1723f06fdc1d486 100644 (file)
--- 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 91485133eb275cb4035664e09dc1fb3bfe7f42a9..e74f48636ec0635abf8561ed8ec08c2ccdea898b 100644 (file)
--- 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 234dd90c1d0a94e98f12cc654e908d54dfa11c41..a4e52d6a5b0e3a717b0fa0c50ee36ceba265a095 100644 (file)
--- 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 a5b9d4400cffa743b9b72b69f37bc7abe34cba6c..7226ec1412875bff384f0c006f28fd73eaf28886 100644 (file)
--- 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;