[PATCH] small refactorings to smbXsrv_open

Michael Adam obnox at samba.org
Fri Feb 26 00:41:46 UTC 2016


In preparation of the multi-channel create
replay patches that I hope to send tomorrow,
here is a small refactoring/consolidation of
the code fetch-locking the local or global
records. I find it useful.

Review appreciated!

Cheers - Michael

-------------- next part --------------
From 64db3feefb2ad5b10605549c906b04478a5ed1d9 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Fri, 26 Feb 2016 00:41:24 +0100
Subject: [PATCH 1/2] smbXsrv_open: factor fetch-locking of global record into
 function

smbXsrv_open_global_fetch_locked()

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/smbd/smbXsrv_open.c | 89 ++++++++++++++++++++-------------------------
 1 file changed, 40 insertions(+), 49 deletions(-)

diff --git a/source3/smbd/smbXsrv_open.c b/source3/smbd/smbXsrv_open.c
index 02addc2..1a0a173 100644
--- a/source3/smbd/smbXsrv_open.c
+++ b/source3/smbd/smbXsrv_open.c
@@ -151,6 +151,31 @@ static NTSTATUS smbXsrv_open_local_key_to_id(TDB_DATA key, uint32_t *id)
 	return NT_STATUS_OK;
 }
 
+static struct db_record *smbXsrv_open_global_fetch_locked(
+			struct db_context *db,
+			uint32_t id,
+			TALLOC_CTX *mem_ctx)
+{
+	TDB_DATA key;
+	uint8_t key_buf[SMBXSRV_OPEN_GLOBAL_TDB_KEY_SIZE];
+	struct db_record *rec = NULL;
+
+	if (db == NULL) {
+		return NULL;
+	}
+
+	key = smbXsrv_open_global_id_to_key(id, key_buf);
+
+	rec = dbwrap_fetch_locked(db, mem_ctx, key);
+
+	if (rec == NULL) {
+		DBG_DEBUG("Failed to lock global id 0x%08x, key '%s'\n", id,
+			  hex_encode_talloc(talloc_tos(), key.dptr, key.dsize));
+	}
+
+	return rec;
+}
+
 static NTSTATUS smbXsrv_open_table_init(struct smbXsrv_connection *conn,
 					uint32_t lowest_id,
 					uint32_t highest_id,
@@ -518,8 +543,6 @@ static NTSTATUS smbXsrv_open_global_allocate(struct db_context *db,
 		bool is_free = false;
 		bool was_free = false;
 		uint32_t id;
-		uint8_t key_buf[SMBXSRV_OPEN_GLOBAL_TDB_KEY_SIZE];
-		TDB_DATA key;
 
 		if (i >= min_tries && last_free != 0) {
 			id = last_free;
@@ -533,9 +556,7 @@ static NTSTATUS smbXsrv_open_global_allocate(struct db_context *db,
 			id--;
 		}
 
-		key = smbXsrv_open_global_id_to_key(id, key_buf);
-
-		global->db_rec = dbwrap_fetch_locked(db, mem_ctx, key);
+		global->db_rec = smbXsrv_open_global_fetch_locked(db, id, mem_ctx);
 		if (global->db_rec == NULL) {
 			talloc_free(global);
 			return NT_STATUS_INSUFFICIENT_RESOURCES;
@@ -741,8 +762,6 @@ static NTSTATUS smbXsrv_open_global_lookup(struct smbXsrv_open_table *table,
 					   TALLOC_CTX *mem_ctx,
 					   struct smbXsrv_open_global0 **_global)
 {
-	TDB_DATA key;
-	uint8_t key_buf[SMBXSRV_OPEN_GLOBAL_TDB_KEY_SIZE];
 	struct db_record *global_rec = NULL;
 	bool is_free = false;
 
@@ -752,15 +771,10 @@ static NTSTATUS smbXsrv_open_global_lookup(struct smbXsrv_open_table *table,
 		return NT_STATUS_INTERNAL_ERROR;
 	}
 
-	key = smbXsrv_open_global_id_to_key(open_global_id, key_buf);
-
-	global_rec = dbwrap_fetch_locked(table->global.db_ctx, mem_ctx, key);
+	global_rec = smbXsrv_open_global_fetch_locked(table->global.db_ctx,
+						      open_global_id,
+						      mem_ctx);
 	if (global_rec == NULL) {
-		DEBUG(0, ("smbXsrv_open_global_lookup(0x%08x): "
-			  "Failed to lock global key '%s'\n",
-			  open_global_id,
-			  hex_encode_talloc(talloc_tos(), key.dptr,
-					    key.dsize)));
 		return NT_STATUS_INTERNAL_DB_ERROR;
 	}
 
@@ -933,8 +947,6 @@ NTSTATUS smbXsrv_open_update(struct smbXsrv_open *op)
 {
 	struct smbXsrv_open_table *table = op->table;
 	NTSTATUS status;
-	uint8_t key_buf[SMBXSRV_OPEN_GLOBAL_TDB_KEY_SIZE];
-	TDB_DATA key;
 
 	if (op->global->db_rec != NULL) {
 		DEBUG(0, ("smbXsrv_open_update(0x%08x): "
@@ -943,17 +955,11 @@ NTSTATUS smbXsrv_open_update(struct smbXsrv_open *op)
 		return NT_STATUS_INTERNAL_ERROR;
 	}
 
-	key = smbXsrv_open_global_id_to_key(op->global->open_global_id,
-					    key_buf);
-
-	op->global->db_rec = dbwrap_fetch_locked(table->global.db_ctx,
-						 op->global, key);
+	op->global->db_rec = smbXsrv_open_global_fetch_locked(
+						table->global.db_ctx,
+						op->global->open_global_id,
+						op->global /* TALLOC_CTX */);
 	if (op->global->db_rec == NULL) {
-		DEBUG(0, ("smbXsrv_open_update(0x%08x): "
-			  "Failed to lock global key '%s'\n",
-			  op->global->open_global_id,
-			  hex_encode_talloc(talloc_tos(), key.dptr,
-					    key.dsize)));
 		return NT_STATUS_INTERNAL_DB_ERROR;
 	}
 
@@ -1003,21 +1009,11 @@ NTSTATUS smbXsrv_open_close(struct smbXsrv_open *op, NTTIME now)
 	global_rec = op->global->db_rec;
 	op->global->db_rec = NULL;
 	if (global_rec == NULL) {
-		uint8_t key_buf[SMBXSRV_OPEN_GLOBAL_TDB_KEY_SIZE];
-		TDB_DATA key;
-
-		key = smbXsrv_open_global_id_to_key(
-						op->global->open_global_id,
-						key_buf);
-
-		global_rec = dbwrap_fetch_locked(table->global.db_ctx,
-						 op->global, key);
+		global_rec = smbXsrv_open_global_fetch_locked(
+					table->global.db_ctx,
+					op->global->open_global_id,
+					op->global /* TALLOC_CTX */);
 		if (global_rec == NULL) {
-			DEBUG(0, ("smbXsrv_open_close(0x%08x): "
-				  "Failed to lock global key '%s'\n",
-				  op->global->open_global_id,
-				  hex_encode_talloc(global_rec, key.dptr,
-						    key.dsize)));
 			error = NT_STATUS_INTERNAL_ERROR;
 		}
 	}
@@ -1427,21 +1423,16 @@ NTSTATUS smbXsrv_open_cleanup(uint64_t persistent_id)
 	NTSTATUS status = NT_STATUS_OK;
 	TALLOC_CTX *frame = talloc_stackframe();
 	struct smbXsrv_open_global0 *op = NULL;
-	uint8_t key_buf[SMBXSRV_OPEN_GLOBAL_TDB_KEY_SIZE];
-	TDB_DATA key;
 	TDB_DATA val;
 	struct db_record *rec;
 	bool delete_open = false;
 	uint32_t global_id = persistent_id & UINT32_MAX;
 
-	key = smbXsrv_open_global_id_to_key(global_id, key_buf);
-	rec = dbwrap_fetch_locked(smbXsrv_open_global_db_ctx, frame, key);
+	rec = smbXsrv_open_global_fetch_locked(smbXsrv_open_global_db_ctx,
+					       global_id,
+					       frame);
 	if (rec == NULL) {
 		status = NT_STATUS_NOT_FOUND;
-		DEBUG(1, ("smbXsrv_open_cleanup[global: 0x%08x] "
-			  "failed to fetch record from %s - %s\n",
-			   global_id, dbwrap_name(smbXsrv_open_global_db_ctx),
-			   nt_errstr(status)));
 		goto done;
 	}
 
-- 
2.5.0


From 49ce1ab7960e3a65bbab6acb3684b0d37de2d834 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Fri, 26 Feb 2016 00:53:22 +0100
Subject: [PATCH 2/2] smbXsrv_open: factor fetch-locking of local record into
 function

smbXsrv_open_local_fetch_locked()

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/smbd/smbXsrv_open.c | 52 ++++++++++++++++++++++++++-------------------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/source3/smbd/smbXsrv_open.c b/source3/smbd/smbXsrv_open.c
index 1a0a173..5bd3da2 100644
--- a/source3/smbd/smbXsrv_open.c
+++ b/source3/smbd/smbXsrv_open.c
@@ -176,6 +176,31 @@ static struct db_record *smbXsrv_open_global_fetch_locked(
 	return rec;
 }
 
+static struct db_record *smbXsrv_open_local_fetch_locked(
+			struct db_context *db,
+			uint32_t id,
+			TALLOC_CTX *mem_ctx)
+{
+	TDB_DATA key;
+	uint8_t key_buf[SMBXSRV_OPEN_LOCAL_TDB_KEY_SIZE];
+	struct db_record *rec = NULL;
+
+	if (db == NULL) {
+		return NULL;
+	}
+
+	key = smbXsrv_open_local_id_to_key(id, key_buf);
+
+	rec = dbwrap_fetch_locked(db, mem_ctx, key);
+
+	if (rec == NULL) {
+		DBG_DEBUG("Failed to lock local id 0x%08x, key '%s'\n", id,
+			  hex_encode_talloc(talloc_tos(), key.dptr, key.dsize));
+	}
+
+	return rec;
+}
+
 static NTSTATUS smbXsrv_open_table_init(struct smbXsrv_connection *conn,
 					uint32_t lowest_id,
 					uint32_t highest_id,
@@ -300,8 +325,6 @@ static NTSTATUS smbXsrv_open_local_allocate_id(struct db_context *db,
 
 	for (i = 0; i < (range / 2); i++) {
 		uint32_t id;
-		uint8_t key_buf[SMBXSRV_OPEN_LOCAL_TDB_KEY_SIZE];
-		TDB_DATA key;
 		TDB_DATA val;
 		struct db_record *rec = NULL;
 
@@ -315,9 +338,7 @@ static NTSTATUS smbXsrv_open_local_allocate_id(struct db_context *db,
 			id = highest_id;
 		}
 
-		key = smbXsrv_open_local_id_to_key(id, key_buf);
-
-		rec = dbwrap_fetch_locked(db, mem_ctx, key);
+		rec = smbXsrv_open_local_fetch_locked(db, id, mem_ctx);
 		if (rec == NULL) {
 			return NT_STATUS_INSUFFICIENT_RESOURCES;
 		}
@@ -367,16 +388,12 @@ static NTSTATUS smbXsrv_open_local_allocate_id(struct db_context *db,
 
 	if (NT_STATUS_IS_OK(state.status)) {
 		uint32_t id;
-		uint8_t key_buf[SMBXSRV_OPEN_LOCAL_TDB_KEY_SIZE];
-		TDB_DATA key;
 		TDB_DATA val;
 		struct db_record *rec = NULL;
 
 		id = state.useable_id;
 
-		key = smbXsrv_open_local_id_to_key(id, key_buf);
-
-		rec = dbwrap_fetch_locked(db, mem_ctx, key);
+		rec = smbXsrv_open_local_fetch_locked(db, id, mem_ctx);
 		if (rec == NULL) {
 			return NT_STATUS_INSUFFICIENT_RESOURCES;
 		}
@@ -1072,19 +1089,10 @@ NTSTATUS smbXsrv_open_close(struct smbXsrv_open *op, NTTIME now)
 
 	local_rec = op->db_rec;
 	if (local_rec == NULL) {
-		uint8_t key_buf[SMBXSRV_OPEN_LOCAL_TDB_KEY_SIZE];
-		TDB_DATA key;
-
-		key = smbXsrv_open_local_id_to_key(op->local_id, key_buf);
-
-		local_rec = dbwrap_fetch_locked(table->local.db_ctx,
-						op, key);
+		local_rec = smbXsrv_open_local_fetch_locked(table->local.db_ctx,
+							    op->local_id,
+							    op /* TALLOC_CTX*/);
 		if (local_rec == NULL) {
-			DEBUG(0, ("smbXsrv_open_close(0x%08x): "
-				  "Failed to lock local key '%s'\n",
-				  op->global->open_global_id,
-				  hex_encode_talloc(local_rec, key.dptr,
-						    key.dsize)));
 			error = NT_STATUS_INTERNAL_ERROR;
 		}
 	}
-- 
2.5.0

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160226/e713bed5/signature.sig>


More information about the samba-technical mailing list