[PATCH] some dbwrap cleanups

Volker Lendecke Volker.Lendecke at SerNet.DE
Mon Sep 21 15:41:36 UTC 2015


Hi!

Review&push appreciated!

Thanks,

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From 02659174d883a2573fa7054db3728e7e694041e2 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 21 Sep 2015 12:28:20 +0200
Subject: [PATCH 1/7] dbwrap: Remove loadparm_context from db_open_tdb

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/dbwrap/dbwrap_local_open.c           |  9 +++++++--
 lib/dbwrap/dbwrap_tdb.c                  |  8 +-------
 lib/dbwrap/dbwrap_tdb.h                  |  1 -
 source4/ntvfs/posix/python/pyxattr_tdb.c | 15 +++++++++++----
 4 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/lib/dbwrap/dbwrap_local_open.c b/lib/dbwrap/dbwrap_local_open.c
index 6509ff9..c350fd3 100644
--- a/lib/dbwrap/dbwrap_local_open.c
+++ b/lib/dbwrap/dbwrap_local_open.c
@@ -34,8 +34,13 @@ struct db_context *dbwrap_local_open(TALLOC_CTX *mem_ctx,
 {
 	struct db_context *db = NULL;
 
-	db = db_open_tdb(mem_ctx, lp_ctx, name, hash_size,
-			 tdb_flags, open_flags, mode,
+	if (hash_size == 0) {
+		hash_size = lpcfg_tdb_hash_size(lp_ctx, name);
+	}
+
+	db = db_open_tdb(mem_ctx, name, hash_size,
+			 lpcfg_tdb_flags(lp_ctx, tdb_flags),
+			 open_flags, mode,
 			 lock_order, dbwrap_flags);
 
 	return db;
diff --git a/lib/dbwrap/dbwrap_tdb.c b/lib/dbwrap/dbwrap_tdb.c
index 2bc123e..574b5d9 100644
--- a/lib/dbwrap/dbwrap_tdb.c
+++ b/lib/dbwrap/dbwrap_tdb.c
@@ -397,7 +397,6 @@ static void db_tdb_id(struct db_context *db, const uint8_t **id, size_t *idlen)
 }
 
 struct db_context *db_open_tdb(TALLOC_CTX *mem_ctx,
-			       struct loadparm_context *lp_ctx,
 			       const char *name,
 			       int hash_size, int tdb_flags,
 			       int open_flags, mode_t mode,
@@ -421,12 +420,7 @@ struct db_context *db_open_tdb(TALLOC_CTX *mem_ctx,
 	}
 	result->lock_order = lock_order;
 
-	if (hash_size == 0) {
-		hash_size = lpcfg_tdb_hash_size(lp_ctx, name);
-	}
-
-	db_tdb->wtdb = tdb_wrap_open(db_tdb, name, hash_size,
-				     lpcfg_tdb_flags(lp_ctx, tdb_flags),
+	db_tdb->wtdb = tdb_wrap_open(db_tdb, name, hash_size, tdb_flags,
 				     open_flags, mode);
 	if (db_tdb->wtdb == NULL) {
 		DEBUG(3, ("Could not open tdb: %s\n", strerror(errno)));
diff --git a/lib/dbwrap/dbwrap_tdb.h b/lib/dbwrap/dbwrap_tdb.h
index 93ee09c..d5f49c7 100644
--- a/lib/dbwrap/dbwrap_tdb.h
+++ b/lib/dbwrap/dbwrap_tdb.h
@@ -25,7 +25,6 @@
 struct db_context;
 
 struct db_context *db_open_tdb(TALLOC_CTX *mem_ctx,
-			       struct loadparm_context *lp_ctx,
 			       const char *name,
 			       int hash_size, int tdb_flags,
 			       int open_flags, mode_t mode,
diff --git a/source4/ntvfs/posix/python/pyxattr_tdb.c b/source4/ntvfs/posix/python/pyxattr_tdb.c
index d3390a3..84ef426 100644
--- a/source4/ntvfs/posix/python/pyxattr_tdb.c
+++ b/source4/ntvfs/posix/python/pyxattr_tdb.c
@@ -46,6 +46,7 @@ static PyObject *py_wrap_setxattr(PyObject *self, PyObject *args)
 	int blobsize;
 	int ret;
 	TALLOC_CTX *mem_ctx;
+	struct loadparm_context *lp_ctx;
 	struct db_context *eadb = NULL;
 	struct file_id id;
 	struct stat sbuf;
@@ -56,8 +57,11 @@ static PyObject *py_wrap_setxattr(PyObject *self, PyObject *args)
 
 	blob.length = blobsize;
 	mem_ctx = talloc_new(NULL);
-	eadb = db_open_tdb(mem_ctx, py_default_loadparm_context(mem_ctx), tdbname, 50000,
-			   TDB_DEFAULT, O_RDWR|O_CREAT, 0600, DBWRAP_LOCK_ORDER_2,
+
+	lp_ctx = py_default_loadparm_context(mem_ctx);
+	eadb = db_open_tdb(mem_ctx, tdbname, 50000,
+			   lpcfg_tdb_flags(lp_ctx, TDB_DEFAULT),
+			   O_RDWR|O_CREAT, 0600, DBWRAP_LOCK_ORDER_2,
 			   DBWRAP_FLAG_NONE);
 
 	if (eadb == NULL) {
@@ -91,6 +95,7 @@ static PyObject *py_wrap_getxattr(PyObject *self, PyObject *args)
 {
 	char *filename, *attribute, *tdbname;
 	TALLOC_CTX *mem_ctx;
+	struct loadparm_context *lp_ctx;
 	DATA_BLOB blob;
 	PyObject *ret_obj;
 	int ret;
@@ -104,8 +109,10 @@ static PyObject *py_wrap_getxattr(PyObject *self, PyObject *args)
 
 	mem_ctx = talloc_new(NULL);
 
-	eadb = db_open_tdb(mem_ctx, py_default_loadparm_context(mem_ctx), tdbname, 50000,
-			   TDB_DEFAULT, O_RDWR|O_CREAT, 0600, DBWRAP_LOCK_ORDER_2,
+	lp_ctx = py_default_loadparm_context(mem_ctx);
+	eadb = db_open_tdb(mem_ctx, tdbname, 50000,
+			   lpcfg_tdb_flags(lp_ctx, TDB_DEFAULT),
+			   O_RDWR|O_CREAT, 0600, DBWRAP_LOCK_ORDER_2,
 			   DBWRAP_FLAG_NONE);
 
 	if (eadb == NULL) {
-- 
1.9.1


From 9aeedcb73b3115123da8ec3937eb12d14dec1ecf Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 20 Sep 2015 20:25:53 +0200
Subject: [PATCH 2/7] dbwrap: Remove unused dbwrap_hash_size()

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/dbwrap/dbwrap.c         | 5 -----
 lib/dbwrap/dbwrap.h         | 1 -
 lib/dbwrap/dbwrap_cache.c   | 1 -
 lib/dbwrap/dbwrap_private.h | 1 -
 lib/dbwrap/dbwrap_tdb.c     | 1 -
 5 files changed, 9 deletions(-)

diff --git a/lib/dbwrap/dbwrap.c b/lib/dbwrap/dbwrap.c
index d75c714..3cfb21b 100644
--- a/lib/dbwrap/dbwrap.c
+++ b/lib/dbwrap/dbwrap.c
@@ -395,11 +395,6 @@ int dbwrap_wipe(struct db_context *db)
 	return db->wipe(db);
 }
 
-int dbwrap_hash_size(struct db_context *db)
-{
-	return db->hash_size;
-}
-
 int dbwrap_check(struct db_context *db)
 {
 	if (db->check == NULL) {
diff --git a/lib/dbwrap/dbwrap.h b/lib/dbwrap/dbwrap.h
index e081e18..9787078 100644
--- a/lib/dbwrap/dbwrap.h
+++ b/lib/dbwrap/dbwrap.h
@@ -83,7 +83,6 @@ int dbwrap_wipe(struct db_context *db);
 int dbwrap_check(struct db_context *db);
 int dbwrap_get_seqnum(struct db_context *db);
 /* Returns 0 if unknown. */
-int dbwrap_hash_size(struct db_context *db);
 int dbwrap_transaction_start(struct db_context *db);
 NTSTATUS dbwrap_transaction_start_nonblock(struct db_context *db);
 int dbwrap_transaction_commit(struct db_context *db);
diff --git a/lib/dbwrap/dbwrap_cache.c b/lib/dbwrap/dbwrap_cache.c
index 724389e..c317fe0 100644
--- a/lib/dbwrap/dbwrap_cache.c
+++ b/lib/dbwrap/dbwrap_cache.c
@@ -222,6 +222,5 @@ struct db_context *db_open_cache(TALLOC_CTX *mem_ctx,
 	db->exists = dbwrap_cache_exists;
 	db->id = dbwrap_cache_id;
 	db->name = dbwrap_name(ctx->backing);
-	db->hash_size = dbwrap_hash_size(ctx->backing);
 	return db;
 }
diff --git a/lib/dbwrap/dbwrap_private.h b/lib/dbwrap/dbwrap_private.h
index a6bad04..f3c42b9 100644
--- a/lib/dbwrap/dbwrap_private.h
+++ b/lib/dbwrap/dbwrap_private.h
@@ -60,7 +60,6 @@ struct db_context {
 	int (*check)(struct db_context *db);
 	void (*id)(struct db_context *db, const uint8_t **id, size_t *idlen);
 	const char *name;
-	int hash_size;
 	void *private_data;
 	enum dbwrap_lock_order lock_order;
 	bool persistent;
diff --git a/lib/dbwrap/dbwrap_tdb.c b/lib/dbwrap/dbwrap_tdb.c
index 574b5d9..3e757ff 100644
--- a/lib/dbwrap/dbwrap_tdb.c
+++ b/lib/dbwrap/dbwrap_tdb.c
@@ -452,7 +452,6 @@ struct db_context *db_open_tdb(TALLOC_CTX *mem_ctx,
 	result->id = db_tdb_id;
 	result->check = db_tdb_check;
 	result->name = tdb_name(db_tdb->wtdb->tdb);
-	result->hash_size = hash_size;
 	return result;
 
  fail:
-- 
1.9.1


From 9e2addbbc2184f6fca957055a41b1e6aa7f6bfe4 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 21 Sep 2015 12:32:47 +0200
Subject: [PATCH 3/7] dbwrap: Remove talloc_reference()

We want to know (by crashing) when we free the database with records still
around. This would be a serious violation of our data structure hierarchies.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/dbwrap/dbwrap_tdb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/dbwrap/dbwrap_tdb.c b/lib/dbwrap/dbwrap_tdb.c
index 3e757ff..a144ed4 100644
--- a/lib/dbwrap/dbwrap_tdb.c
+++ b/lib/dbwrap/dbwrap_tdb.c
@@ -136,7 +136,7 @@ static struct db_record *db_tdb_fetch_locked_internal(
 
 	talloc_set_destructor(state.result, db_tdb_record_destr);
 
-	state.result->private_data = talloc_reference(state.result, ctx);
+	state.result->private_data = ctx;
 	state.result->store = db_tdb_store;
 	state.result->delete_rec = db_tdb_delete;
 
-- 
1.9.1


From 06799df610b96047f033c35219d491c3c0d1bd15 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 20 Sep 2015 16:26:06 +0200
Subject: [PATCH 4/7] dbwrap: Make dbwrap_db_id return size_t

This will make an on-stack db-id easier

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/dbwrap/dbwrap.c               |  4 ++--
 lib/dbwrap/dbwrap.h               |  2 +-
 lib/dbwrap/dbwrap_cache.c         |  7 ++++---
 lib/dbwrap/dbwrap_private.h       |  3 ++-
 lib/dbwrap/dbwrap_rbt.c           |  8 +++++---
 lib/dbwrap/dbwrap_tdb.c           | 10 +++++++---
 source3/lib/dbwrap/dbwrap_ctdb.c  | 10 ++++++----
 source3/lib/dbwrap/dbwrap_watch.c |  7 ++++---
 8 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/lib/dbwrap/dbwrap.c b/lib/dbwrap/dbwrap.c
index 3cfb21b..a1b98c3 100644
--- a/lib/dbwrap/dbwrap.c
+++ b/lib/dbwrap/dbwrap.c
@@ -449,9 +449,9 @@ int dbwrap_transaction_cancel(struct db_context *db)
 	return db->transaction_cancel(db);
 }
 
-void dbwrap_db_id(struct db_context *db, const uint8_t **id, size_t *idlen)
+size_t dbwrap_db_id(struct db_context *db, uint8_t *id, size_t idlen)
 {
-	db->id(db, id, idlen);
+	return db->id(db, id, idlen);
 }
 
 bool dbwrap_is_persistent(struct db_context *db)
diff --git a/lib/dbwrap/dbwrap.h b/lib/dbwrap/dbwrap.h
index 9787078..0a5c918 100644
--- a/lib/dbwrap/dbwrap.h
+++ b/lib/dbwrap/dbwrap.h
@@ -87,7 +87,7 @@ int dbwrap_transaction_start(struct db_context *db);
 NTSTATUS dbwrap_transaction_start_nonblock(struct db_context *db);
 int dbwrap_transaction_commit(struct db_context *db);
 int dbwrap_transaction_cancel(struct db_context *db);
-void dbwrap_db_id(struct db_context *db, const uint8_t **id, size_t *idlen);
+size_t dbwrap_db_id(struct db_context *db, uint8_t *id, size_t idlen);
 bool dbwrap_is_persistent(struct db_context *db);
 const char *dbwrap_name(struct db_context *db);
 
diff --git a/lib/dbwrap/dbwrap_cache.c b/lib/dbwrap/dbwrap_cache.c
index c317fe0..e4cee55 100644
--- a/lib/dbwrap/dbwrap_cache.c
+++ b/lib/dbwrap/dbwrap_cache.c
@@ -179,12 +179,13 @@ static int dbwrap_cache_exists(struct db_context *db, TDB_DATA key)
 	return dbwrap_exists(ctx->backing, key);
 }
 
-static void dbwrap_cache_id(struct db_context *db, const uint8_t **id,
-			    size_t *idlen)
+static size_t dbwrap_cache_id(struct db_context *db, uint8_t *id,
+			      size_t idlen)
 {
 	struct db_cache_ctx *ctx = talloc_get_type_abort(
 		db->private_data, struct db_cache_ctx);
-	dbwrap_db_id(ctx->backing, id, idlen);
+
+	return dbwrap_db_id(ctx->backing, id, idlen);
 }
 
 struct db_context *db_open_cache(TALLOC_CTX *mem_ctx,
diff --git a/lib/dbwrap/dbwrap_private.h b/lib/dbwrap/dbwrap_private.h
index f3c42b9..6a52850 100644
--- a/lib/dbwrap/dbwrap_private.h
+++ b/lib/dbwrap/dbwrap_private.h
@@ -58,7 +58,8 @@ struct db_context {
 	int (*exists)(struct db_context *db,TDB_DATA key);
 	int (*wipe)(struct db_context *db);
 	int (*check)(struct db_context *db);
-	void (*id)(struct db_context *db, const uint8_t **id, size_t *idlen);
+	size_t (*id)(struct db_context *db, uint8_t *id, size_t idlen);
+
 	const char *name;
 	void *private_data;
 	enum dbwrap_lock_order lock_order;
diff --git a/lib/dbwrap/dbwrap_rbt.c b/lib/dbwrap/dbwrap_rbt.c
index 03f2f57..0764a2c 100644
--- a/lib/dbwrap/dbwrap_rbt.c
+++ b/lib/dbwrap/dbwrap_rbt.c
@@ -497,10 +497,12 @@ static int db_rbt_trans_dummy(struct db_context *db)
 	return 0;
 }
 
-static void db_rbt_id(struct db_context *db, const uint8_t **id, size_t *idlen)
+static size_t db_rbt_id(struct db_context *db, uint8_t *id, size_t idlen)
 {
-	*id = (uint8_t *)db;
-	*idlen = sizeof(struct db_context *);
+	if (idlen >= sizeof(struct db_context *)) {
+		memcpy(id, &db, sizeof(struct db_context *));
+	}
+	return sizeof(struct db_context *);
 }
 
 struct db_context *db_open_rbt(TALLOC_CTX *mem_ctx)
diff --git a/lib/dbwrap/dbwrap_tdb.c b/lib/dbwrap/dbwrap_tdb.c
index a144ed4..0e54449 100644
--- a/lib/dbwrap/dbwrap_tdb.c
+++ b/lib/dbwrap/dbwrap_tdb.c
@@ -388,12 +388,16 @@ static int db_tdb_transaction_cancel(struct db_context *db)
 	return 0;
 }
 
-static void db_tdb_id(struct db_context *db, const uint8_t **id, size_t *idlen)
+static size_t db_tdb_id(struct db_context *db, uint8_t *id, size_t idlen)
 {
 	struct db_tdb_ctx *db_ctx =
 		talloc_get_type_abort(db->private_data, struct db_tdb_ctx);
-	*id = (uint8_t *)&db_ctx->id;
-	*idlen = sizeof(db_ctx->id);
+
+	if (idlen >= sizeof(db_ctx->id)) {
+		memcpy(id, &db_ctx->id, sizeof(db_ctx->id));
+	}
+
+	return sizeof(db_ctx->id);
 }
 
 struct db_context *db_open_tdb(TALLOC_CTX *mem_ctx,
diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index f37bfd8..3b68338 100644
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -1517,14 +1517,16 @@ static int db_ctdb_get_seqnum(struct db_context *db)
 	return tdb_get_seqnum(ctx->wtdb->tdb);
 }
 
-static void db_ctdb_id(struct db_context *db, const uint8_t **id,
-		       size_t *idlen)
+static size_t db_ctdb_id(struct db_context *db, uint8_t *id, size_t idlen)
 {
 	struct db_ctdb_ctx *ctx = talloc_get_type_abort(
 		db->private_data, struct db_ctdb_ctx);
 
-	*id = (uint8_t *)&ctx->db_id;
-	*idlen = sizeof(ctx->db_id);
+	if (idlen >= sizeof(ctx->db_id)) {
+		memcpy(id, &ctx->db_id, sizeof(ctx->db_id));
+	}
+
+	return sizeof(ctx->db_id);
 }
 
 struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
diff --git a/source3/lib/dbwrap/dbwrap_watch.c b/source3/lib/dbwrap/dbwrap_watch.c
index da1a9cc..11ade19 100644
--- a/source3/lib/dbwrap/dbwrap_watch.c
+++ b/source3/lib/dbwrap/dbwrap_watch.c
@@ -50,11 +50,12 @@ static TDB_DATA dbwrap_record_watchers_key(TALLOC_CTX *mem_ctx,
 					   struct db_record *rec,
 					   TDB_DATA *rec_key)
 {
-	const uint8_t *db_id;
-	size_t db_id_len;
+	size_t db_id_len = dbwrap_db_id(db, NULL, 0);
+	uint8_t db_id[db_id_len];
 	TDB_DATA key, wkey;
 
-	dbwrap_db_id(db, &db_id, &db_id_len);
+	dbwrap_db_id(db, db_id, db_id_len);
+
 	key = dbwrap_record_get_key(rec);
 
 	wkey.dsize = sizeof(uint32_t) + db_id_len + key.dsize;
-- 
1.9.1


From 846c184c7fc5415b1b75098d64089a61a9f914be Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 20 Sep 2015 17:06:22 +0200
Subject: [PATCH 5/7] dbwrap: Simplify dbwrap_record_watchers_key()

It took a bit for me to figure out what the rec_key parameter to
dbwrap_record_watchers_key does. I think it's simpler to parse the watcher key
in dbwrap_record_watch_recv to retrieve the watched record key than to store it

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/dbwrap/dbwrap_watch.c | 40 ++++++++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/source3/lib/dbwrap/dbwrap_watch.c b/source3/lib/dbwrap/dbwrap_watch.c
index 11ade19..419b009 100644
--- a/source3/lib/dbwrap/dbwrap_watch.c
+++ b/source3/lib/dbwrap/dbwrap_watch.c
@@ -47,8 +47,7 @@ static struct db_context *dbwrap_record_watchers_db(void)
 
 static TDB_DATA dbwrap_record_watchers_key(TALLOC_CTX *mem_ctx,
 					   struct db_context *db,
-					   struct db_record *rec,
-					   TDB_DATA *rec_key)
+					   struct db_record *rec)
 {
 	size_t db_id_len = dbwrap_db_id(db, NULL, 0);
 	uint8_t db_id[db_id_len];
@@ -67,11 +66,6 @@ static TDB_DATA dbwrap_record_watchers_key(TALLOC_CTX *mem_ctx,
 	memcpy(wkey.dptr + sizeof(uint32_t), db_id, db_id_len);
 	memcpy(wkey.dptr + sizeof(uint32_t) + db_id_len, key.dptr, key.dsize);
 
-	if (rec_key != NULL) {
-		rec_key->dptr = wkey.dptr + sizeof(uint32_t) + db_id_len;
-		rec_key->dsize = key.dsize;
-	}
-
 	return wkey;
 }
 
@@ -90,10 +84,16 @@ static bool dbwrap_record_watchers_key_parse(
 			  "db_id_len=%d\n", (int)wkey.dsize, (int)db_id_len));
 		return false;
 	}
-	*p_db_id = wkey.dptr + sizeof(uint32_t);
-	*p_db_id_len = db_id_len;
-	key->dptr = wkey.dptr + sizeof(uint32_t) + db_id_len;
-	key->dsize = wkey.dsize - sizeof(uint32_t) - db_id_len;
+	if (p_db_id != NULL) {
+		*p_db_id = wkey.dptr + sizeof(uint32_t);
+	}
+	if (p_db_id_len != NULL) {
+		*p_db_id_len = db_id_len;
+	}
+	if (key != NULL) {
+		key->dptr = wkey.dptr + sizeof(uint32_t) + db_id_len;
+		key->dsize = wkey.dsize - sizeof(uint32_t) - db_id_len;
+	}
 	return true;
 }
 
@@ -204,7 +204,7 @@ static NTSTATUS dbwrap_record_get_watchers(struct db_context *db,
 	struct server_id *ids;
 	NTSTATUS status;
 
-	key = dbwrap_record_watchers_key(talloc_tos(), db, rec, NULL);
+	key = dbwrap_record_watchers_key(talloc_tos(), db, rec);
 	if (key.dptr == NULL) {
 		status = NT_STATUS_NO_MEMORY;
 		goto fail;
@@ -238,7 +238,6 @@ struct dbwrap_record_watch_state {
 	struct db_context *db;
 	struct tevent_req *req;
 	struct messaging_context *msg;
-	TDB_DATA key;
 	TDB_DATA w_key;
 };
 
@@ -274,8 +273,7 @@ struct tevent_req *dbwrap_record_watch_send(TALLOC_CTX *mem_ctx,
 		return tevent_req_post(req, ev);
 	}
 
-	state->w_key = dbwrap_record_watchers_key(state, state->db, rec,
-						  &state->key);
+	state->w_key = dbwrap_record_watchers_key(state, state->db, rec);
 	if (tevent_req_nomem(state->w_key.dptr, req)) {
 		return tevent_req_post(req, ev);
 	}
@@ -347,7 +345,7 @@ static void dbwrap_watch_record_stored(struct db_context *db,
 			  nt_errstr(status)));
 		goto done;
 	}
-	w_key = dbwrap_record_watchers_key(talloc_tos(), db, rec, NULL);
+	w_key = dbwrap_record_watchers_key(talloc_tos(), db, rec);
 	if (w_key.dptr == NULL) {
 		DEBUG(1, ("dbwrap_record_watchers_key failed\n"));
 		goto done;
@@ -397,7 +395,9 @@ NTSTATUS dbwrap_record_watch_recv(struct tevent_req *req,
 	struct dbwrap_record_watch_state *state = tevent_req_data(
 		req, struct dbwrap_record_watch_state);
 	NTSTATUS status;
+	TDB_DATA key;
 	struct db_record *rec;
+	bool ok;
 
 	if (tevent_req_is_nterror(req, &status)) {
 		return status;
@@ -405,7 +405,13 @@ NTSTATUS dbwrap_record_watch_recv(struct tevent_req *req,
 	if (prec == NULL) {
 		return NT_STATUS_OK;
 	}
-	rec = dbwrap_fetch_locked(state->db, mem_ctx, state->key);
+
+	ok = dbwrap_record_watchers_key_parse(state->w_key, NULL, NULL, &key);
+	if (!ok) {
+		return NT_STATUS_INTERNAL_DB_ERROR;
+	}
+
+	rec = dbwrap_fetch_locked(state->db, mem_ctx, key);
 	if (rec == NULL) {
 		return NT_STATUS_INTERNAL_DB_ERROR;
 	}
-- 
1.9.1


From e42302f19e5bd79eab91b3401ad4d13b3745f9bb Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 20 Sep 2015 17:32:24 +0200
Subject: [PATCH 6/7] dbwrap: Convert dbwrap_record_watchers_key to not use
 talloc

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/dbwrap/dbwrap_watch.c | 63 ++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 30 deletions(-)

diff --git a/source3/lib/dbwrap/dbwrap_watch.c b/source3/lib/dbwrap/dbwrap_watch.c
index 419b009..1540e0b 100644
--- a/source3/lib/dbwrap/dbwrap_watch.c
+++ b/source3/lib/dbwrap/dbwrap_watch.c
@@ -45,28 +45,29 @@ static struct db_context *dbwrap_record_watchers_db(void)
 	return watchers_db;
 }
 
-static TDB_DATA dbwrap_record_watchers_key(TALLOC_CTX *mem_ctx,
-					   struct db_context *db,
-					   struct db_record *rec)
+static size_t dbwrap_record_watchers_key(struct db_context *db,
+					 struct db_record *rec,
+					 uint8_t *wkey, size_t wkey_len)
 {
 	size_t db_id_len = dbwrap_db_id(db, NULL, 0);
 	uint8_t db_id[db_id_len];
-	TDB_DATA key, wkey;
+	size_t needed;
+	TDB_DATA key;
 
 	dbwrap_db_id(db, db_id, db_id_len);
 
 	key = dbwrap_record_get_key(rec);
 
-	wkey.dsize = sizeof(uint32_t) + db_id_len + key.dsize;
-	wkey.dptr = talloc_array(mem_ctx, uint8_t, wkey.dsize);
-	if (wkey.dptr == NULL) {
-		return make_tdb_data(NULL, 0);
+	needed = sizeof(uint32_t) + db_id_len + key.dsize;
+
+	if (wkey_len >= needed) {
+		SIVAL(wkey, 0, db_id_len);
+		memcpy(wkey + sizeof(uint32_t), db_id, db_id_len);
+		memcpy(wkey + sizeof(uint32_t) + db_id_len,
+		       key.dptr, key.dsize);
 	}
-	SIVAL(wkey.dptr, 0, db_id_len);
-	memcpy(wkey.dptr + sizeof(uint32_t), db_id, db_id_len);
-	memcpy(wkey.dptr + sizeof(uint32_t) + db_id_len, key.dptr, key.dsize);
 
-	return wkey;
+	return needed;
 }
 
 static bool dbwrap_record_watchers_key_parse(
@@ -199,23 +200,21 @@ static NTSTATUS dbwrap_record_get_watchers(struct db_context *db,
 					   size_t *p_num_ids)
 {
 	struct db_context *w_db;
-	TDB_DATA key = { 0, };
+	size_t wkey_len = dbwrap_record_watchers_key(db, rec, NULL, 0);
+	uint8_t wkey_buf[wkey_len];
+	TDB_DATA wkey = { .dptr = wkey_buf, .dsize = wkey_len };
 	TDB_DATA value = { 0, };
 	struct server_id *ids;
 	NTSTATUS status;
 
-	key = dbwrap_record_watchers_key(talloc_tos(), db, rec);
-	if (key.dptr == NULL) {
-		status = NT_STATUS_NO_MEMORY;
-		goto fail;
-	}
+	dbwrap_record_watchers_key(db, rec, wkey_buf, wkey_len);
+
 	w_db = dbwrap_record_watchers_db();
 	if (w_db == NULL) {
 		status = NT_STATUS_INTERNAL_ERROR;
 		goto fail;
 	}
-	status = dbwrap_fetch(w_db, mem_ctx, key, &value);
-	TALLOC_FREE(key.dptr);
+	status = dbwrap_fetch(w_db, mem_ctx, wkey, &value);
 	if (!NT_STATUS_IS_OK(status)) {
 		goto fail;
 	}
@@ -228,7 +227,6 @@ static NTSTATUS dbwrap_record_get_watchers(struct db_context *db,
 	*p_num_ids = value.dsize / sizeof(struct server_id);
 	return NT_STATUS_OK;
 fail:
-	TALLOC_FREE(key.dptr);
 	TALLOC_FREE(value.dptr);
 	return status;
 }
@@ -273,10 +271,15 @@ struct tevent_req *dbwrap_record_watch_send(TALLOC_CTX *mem_ctx,
 		return tevent_req_post(req, ev);
 	}
 
-	state->w_key = dbwrap_record_watchers_key(state, state->db, rec);
+	state->w_key.dsize = dbwrap_record_watchers_key(
+		state->db, rec, NULL, 0);
+
+	state->w_key.dptr = talloc_array(state, uint8_t, state->w_key.dsize);
 	if (tevent_req_nomem(state->w_key.dptr, req)) {
 		return tevent_req_post(req, ev);
 	}
+	dbwrap_record_watchers_key(
+		state->db, rec, state->w_key.dptr, state->w_key.dsize);
 
 	subreq = messaging_filtered_read_send(
 		state, ev, state->msg, dbwrap_record_watch_filter, state);
@@ -331,10 +334,16 @@ static void dbwrap_watch_record_stored(struct db_context *db,
 		private_data, struct messaging_context);
 	struct server_id *ids = NULL;
 	size_t num_ids = 0;
-	TDB_DATA w_key = { 0, };
+
+	size_t wkey_len = dbwrap_record_watchers_key(db, rec, NULL, 0);
+	uint8_t wkey_buf[wkey_len];
+	TDB_DATA wkey = { .dptr = wkey_buf, .dsize = wkey_len };
+
 	NTSTATUS status;
 	uint32_t i;
 
+	dbwrap_record_watchers_key(db, rec, wkey_buf, wkey_len);
+
 	status = dbwrap_record_get_watchers(db, rec, talloc_tos(),
 					    &ids, &num_ids);
 	if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
@@ -345,15 +354,10 @@ static void dbwrap_watch_record_stored(struct db_context *db,
 			  nt_errstr(status)));
 		goto done;
 	}
-	w_key = dbwrap_record_watchers_key(talloc_tos(), db, rec);
-	if (w_key.dptr == NULL) {
-		DEBUG(1, ("dbwrap_record_watchers_key failed\n"));
-		goto done;
-	}
 
 	for (i=0; i<num_ids; i++) {
 		status = messaging_send_buf(msg, ids[i], MSG_DBWRAP_MODIFIED,
-					    w_key.dptr, w_key.dsize);
+					    wkey.dptr, wkey.dsize);
 		if (!NT_STATUS_IS_OK(status)) {
 			struct server_id_buf tmp;
 			DEBUG(1, ("messaging_send to %s failed: %s\n",
@@ -362,7 +366,6 @@ static void dbwrap_watch_record_stored(struct db_context *db,
 		}
 	}
 done:
-	TALLOC_FREE(w_key.dptr);
 	TALLOC_FREE(ids);
 	return;
 }
-- 
1.9.1


From 3637f7b0ef6dfc93c9106d752f464c20b570d679 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 20 Sep 2015 18:25:20 +0200
Subject: [PATCH 7/7] dbwrap: Remove talloc from dbwrap_watch_record_stored()

This happens on every store to locking.tdb for example, so we should
make it cheap.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/dbwrap/dbwrap_watch.c | 103 ++++++++++++++++----------------------
 1 file changed, 42 insertions(+), 61 deletions(-)

diff --git a/source3/lib/dbwrap/dbwrap_watch.c b/source3/lib/dbwrap/dbwrap_watch.c
index 1540e0b..426fe77 100644
--- a/source3/lib/dbwrap/dbwrap_watch.c
+++ b/source3/lib/dbwrap/dbwrap_watch.c
@@ -193,44 +193,6 @@ done:
 	return status;
 }
 
-static NTSTATUS dbwrap_record_get_watchers(struct db_context *db,
-					   struct db_record *rec,
-					   TALLOC_CTX *mem_ctx,
-					   struct server_id **p_ids,
-					   size_t *p_num_ids)
-{
-	struct db_context *w_db;
-	size_t wkey_len = dbwrap_record_watchers_key(db, rec, NULL, 0);
-	uint8_t wkey_buf[wkey_len];
-	TDB_DATA wkey = { .dptr = wkey_buf, .dsize = wkey_len };
-	TDB_DATA value = { 0, };
-	struct server_id *ids;
-	NTSTATUS status;
-
-	dbwrap_record_watchers_key(db, rec, wkey_buf, wkey_len);
-
-	w_db = dbwrap_record_watchers_db();
-	if (w_db == NULL) {
-		status = NT_STATUS_INTERNAL_ERROR;
-		goto fail;
-	}
-	status = dbwrap_fetch(w_db, mem_ctx, wkey, &value);
-	if (!NT_STATUS_IS_OK(status)) {
-		goto fail;
-	}
-	if ((value.dsize % sizeof(struct server_id)) != 0) {
-		status = NT_STATUS_INTERNAL_DB_CORRUPTION;
-		goto fail;
-	}
-	ids = (struct server_id *)value.dptr;
-	*p_ids = talloc_move(mem_ctx, &ids);
-	*p_num_ids = value.dsize / sizeof(struct server_id);
-	return NT_STATUS_OK;
-fail:
-	TALLOC_FREE(value.dptr);
-	return status;
-}
-
 struct dbwrap_record_watch_state {
 	struct tevent_context *ev;
 	struct db_context *db;
@@ -326,48 +288,67 @@ static int dbwrap_record_watch_state_destructor(
 	return 0;
 }
 
+static void dbwrap_watch_record_stored_fn(TDB_DATA key, TDB_DATA data,
+					  void *private_data)
+{
+	struct messaging_context *msg = private_data;
+	size_t i, num_ids;
+
+	if ((data.dsize % sizeof(struct server_id)) != 0) {
+		DBG_WARNING("%s: Invalid data size: %zu\n", __func__,
+			    data.dsize);
+		return;
+	}
+	num_ids = data.dsize / sizeof(struct server_id);
+
+	for (i=0; i<num_ids; i++) {
+		struct server_id dst;
+		NTSTATUS status;
+
+		memcpy(&dst, data.dptr + i * sizeof(struct server_id),
+		       sizeof(struct server_id));
+
+		status = messaging_send_buf(msg, dst, MSG_DBWRAP_MODIFIED,
+					    key.dptr, key.dsize);
+		if (!NT_STATUS_IS_OK(status)) {
+			struct server_id_buf tmp;
+			DBG_WARNING("%s: messaging_send to %s failed: %s\n",
+				    __func__, server_id_str_buf(dst, &tmp),
+				    nt_errstr(status));
+		}
+	}
+}
+
 static void dbwrap_watch_record_stored(struct db_context *db,
 				       struct db_record *rec,
 				       void *private_data)
 {
 	struct messaging_context *msg = talloc_get_type_abort(
 		private_data, struct messaging_context);
-	struct server_id *ids = NULL;
-	size_t num_ids = 0;
+	struct db_context *watchers_db;
 
 	size_t wkey_len = dbwrap_record_watchers_key(db, rec, NULL, 0);
 	uint8_t wkey_buf[wkey_len];
 	TDB_DATA wkey = { .dptr = wkey_buf, .dsize = wkey_len };
 
 	NTSTATUS status;
-	uint32_t i;
+
+	watchers_db = dbwrap_record_watchers_db();
+	if (watchers_db == NULL) {
+		return;
+	}
 
 	dbwrap_record_watchers_key(db, rec, wkey_buf, wkey_len);
 
-	status = dbwrap_record_get_watchers(db, rec, talloc_tos(),
-					    &ids, &num_ids);
+	status = dbwrap_parse_record(watchers_db, wkey,
+				     dbwrap_watch_record_stored_fn, msg);
 	if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
-		goto done;
+		return;
 	}
 	if (!NT_STATUS_IS_OK(status)) {
-		DEBUG(1, ("dbwrap_record_get_watchers failed: %s\n",
-			  nt_errstr(status)));
-		goto done;
+		DBG_WARNING("%s: dbwrap_parse_record failed: %s\n",
+			    __func__, nt_errstr(status));
 	}
-
-	for (i=0; i<num_ids; i++) {
-		status = messaging_send_buf(msg, ids[i], MSG_DBWRAP_MODIFIED,
-					    wkey.dptr, wkey.dsize);
-		if (!NT_STATUS_IS_OK(status)) {
-			struct server_id_buf tmp;
-			DEBUG(1, ("messaging_send to %s failed: %s\n",
-				  server_id_str_buf(ids[i], &tmp),
-				  nt_errstr(status)));
-		}
-	}
-done:
-	TALLOC_FREE(ids);
-	return;
 }
 
 void dbwrap_watch_db(struct db_context *db, struct messaging_context *msg)
-- 
1.9.1



More information about the samba-technical mailing list