[SCM] Samba Shared Repository - branch master updated

Volker Lendecke vlendec at samba.org
Sat Mar 13 12:21:11 MST 2010


The branch, master has been updated
       via  b4826b9... s3: Convert unexpected.tdb to use tdb_wrap_open
       via  cfc44d2... s3: Make tdb_wrap_open more robust
      from  5eeb1fc... NSS:winbind_struct_protocol.h - fix typo

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit b4826b9393244e46e855a65a65df67d9e6c1d81c
Author: Volker Lendecke <vl at samba.org>
Date:   Sat Mar 13 20:02:16 2010 +0100

    s3: Convert unexpected.tdb to use tdb_wrap_open

commit cfc44d244152609e17a26db85bbbf827955958a7
Author: Volker Lendecke <vl at samba.org>
Date:   Sat Mar 13 19:05:38 2010 +0100

    s3: Make tdb_wrap_open more robust
    
    This hides the use of talloc_reference from the caller, making it impossible to
    wrongly call talloc_free() on the result.

-----------------------------------------------------------------------

Summary of changes:
 source3/include/util_tdb.h  |    2 -
 source3/lib/util_tdb.c      |  128 +++++++++++++++++++++++++++++--------------
 source3/libsmb/unexpected.c |   22 ++++---
 3 files changed, 99 insertions(+), 53 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/include/util_tdb.h b/source3/include/util_tdb.h
index 80b9592..9666fc9 100644
--- a/source3/include/util_tdb.h
+++ b/source3/include/util_tdb.h
@@ -28,8 +28,6 @@
 
 struct tdb_wrap {
 	struct tdb_context *tdb;
-	const char *name;
-	struct tdb_wrap *next, *prev;
 };
 
 int tdb_chainlock_with_timeout( struct tdb_context *tdb, TDB_DATA key,
diff --git a/source3/lib/util_tdb.c b/source3/lib/util_tdb.c
index af0573e..fe2f231 100644
--- a/source3/lib/util_tdb.c
+++ b/source3/lib/util_tdb.c
@@ -523,75 +523,121 @@ static void tdb_wrap_log(TDB_CONTEXT *tdb, enum tdb_debug_level level,
 	}
 }
 
-static struct tdb_wrap *tdb_list;
+struct tdb_wrap_private {
+	struct tdb_context *tdb;
+	const char *name;
+	struct tdb_wrap_private *next, *prev;
+};
+
+static struct tdb_wrap_private *tdb_list;
 
 /* destroy the last connection to a tdb */
-static int tdb_wrap_destructor(struct tdb_wrap *w)
+static int tdb_wrap_private_destructor(struct tdb_wrap_private *w)
 {
 	tdb_close(w->tdb);
 	DLIST_REMOVE(tdb_list, w);
 	return 0;
 }				 
 
-/*
-  wrapped connection to a tdb database
-  to close just talloc_free() the tdb_wrap pointer
- */
-struct tdb_wrap *tdb_wrap_open(TALLOC_CTX *mem_ctx,
-			       const char *name, int hash_size, int tdb_flags,
-			       int open_flags, mode_t mode)
+static struct tdb_wrap_private *tdb_wrap_private_open(TALLOC_CTX *mem_ctx,
+						      const char *name,
+						      int hash_size,
+						      int tdb_flags,
+						      int open_flags,
+						      mode_t mode)
 {
-	struct tdb_wrap *w;
+	struct tdb_wrap_private *result;
 	struct tdb_logging_context log_ctx;
-	log_ctx.log_fn = tdb_wrap_log;
-
-	if (!lp_use_mmap())
-		tdb_flags |= TDB_NOMMAP;
-
-	for (w=tdb_list;w;w=w->next) {
-		if (strcmp(name, w->name) == 0) {
-			/*
-			 * Yes, talloc_reference is exactly what we want
-			 * here. Otherwise we would have to implement our own
-			 * reference counting.
-			 */
-			return talloc_reference(mem_ctx, w);
-		}
-	}
 
-	w = talloc(mem_ctx, struct tdb_wrap);
-	if (w == NULL) {
+	result = talloc(mem_ctx, struct tdb_wrap_private);
+	if (result == NULL) {
 		return NULL;
 	}
+	result->name = talloc_strdup(result, name);
+	if (result->name == NULL) {
+		goto fail;
+	}
 
-	if (!(w->name = talloc_strdup(w, name))) {
-		talloc_free(w);
-		return NULL;
+	log_ctx.log_fn = tdb_wrap_log;
+
+	if (!lp_use_mmap()) {
+		tdb_flags |= TDB_NOMMAP;
 	}
 
 	if ((hash_size == 0) && (name != NULL)) {
-		const char *base = strrchr_m(name, '/');
+		const char *base;
+		base = strrchr_m(name, '/');
+
 		if (base != NULL) {
 			base += 1;
-		}
-		else {
+		} else {
 			base = name;
 		}
 		hash_size = lp_parm_int(-1, "tdb_hashsize", base, 0);
 	}
 
-	w->tdb = tdb_open_ex(name, hash_size, tdb_flags, 
-			     open_flags, mode, &log_ctx, NULL);
-	if (w->tdb == NULL) {
-		talloc_free(w);
-		return NULL;
+	result->tdb = tdb_open_ex(name, hash_size, tdb_flags,
+				  open_flags, mode, &log_ctx, NULL);
+	if (result->tdb == NULL) {
+		goto fail;
 	}
+	talloc_set_destructor(result, tdb_wrap_private_destructor);
+	DLIST_ADD(tdb_list, result);
+	return result;
+
+fail:
+	TALLOC_FREE(result);
+	return NULL;
+}
+
+/*
+  wrapped connection to a tdb database
+  to close just talloc_free() the tdb_wrap pointer
+ */
+struct tdb_wrap *tdb_wrap_open(TALLOC_CTX *mem_ctx,
+			       const char *name, int hash_size, int tdb_flags,
+			       int open_flags, mode_t mode)
+{
+	struct tdb_wrap *result;
+	struct tdb_wrap_private *w;
 
-	talloc_set_destructor(w, tdb_wrap_destructor);
+	result = talloc(mem_ctx, struct tdb_wrap);
+	if (result == NULL) {
+		return NULL;
+	}
 
-	DLIST_ADD(tdb_list, w);
+	for (w=tdb_list;w;w=w->next) {
+		if (strcmp(name, w->name) == 0) {
+			break;
+		}
+	}
 
-	return w;
+	if (w == NULL) {
+		w = tdb_wrap_private_open(result, name, hash_size, tdb_flags,
+					  open_flags, mode);
+	} else {
+		/*
+		 * Correctly use talloc_reference: The tdb will be
+		 * closed when "w" is being freed. The caller never
+		 * sees "w", so an incorrect use of talloc_free(w)
+		 * instead of calling talloc_unlink is not possible.
+		 * To avoid having to refcount ourselves, "w" will
+		 * have multiple parents that hang off all the
+		 * tdb_wrap's being returned from here. Those parents
+		 * can be freed without problem.
+		 */
+		if (talloc_reference(result, w) == NULL) {
+			goto fail;
+		}
+	}
+	if (w == NULL) {
+		goto fail;
+	}
+	result->tdb = w->tdb;
+	return result;
+fail:
+	TALLOC_FREE(result);
+	return NULL;
 }
 
 NTSTATUS map_nt_error_from_tdb(enum TDB_ERROR err)
diff --git a/source3/libsmb/unexpected.c b/source3/libsmb/unexpected.c
index d123e24..1ac45ec 100644
--- a/source3/libsmb/unexpected.c
+++ b/source3/libsmb/unexpected.c
@@ -20,7 +20,7 @@
 
 #include "includes.h"
 
-static TDB_CONTEXT *tdbd = NULL;
+static struct tdb_wrap *tdbd = NULL;
 
 /* the key type used in the unexpected packet database */
 struct unexpected_key {
@@ -45,9 +45,10 @@ void unexpected_packet(struct packet_struct *p)
 	uint32_t enc_ip;
 
 	if (!tdbd) {
-		tdbd = tdb_open_log(lock_path("unexpected.tdb"), 0,
-			       TDB_CLEAR_IF_FIRST|TDB_DEFAULT,
-			       O_RDWR | O_CREAT, 0644);
+		tdbd = tdb_wrap_open(talloc_autofree_context(),
+				     lock_path("unexpected.tdb"), 0,
+				     TDB_CLEAR_IF_FIRST|TDB_DEFAULT,
+				     O_RDWR | O_CREAT, 0644);
 		if (!tdbd) {
 			DEBUG(0,("Failed to open unexpected.tdb\n"));
 			return;
@@ -74,7 +75,7 @@ void unexpected_packet(struct packet_struct *p)
 	dbuf.dptr = (uint8_t *)buf;
 	dbuf.dsize = len;
 
-	tdb_store(tdbd, kbuf, dbuf, TDB_REPLACE);
+	tdb_store(tdbd->tdb, kbuf, dbuf, TDB_REPLACE);
 }
 
 
@@ -115,7 +116,7 @@ void clear_unexpected(time_t t)
 
 	lastt = t;
 
-	tdb_traverse(tdbd, traverse_fn, NULL);
+	tdb_traverse(tdbd->tdb, traverse_fn, NULL);
 }
 
 struct receive_unexpected_state {
@@ -185,10 +186,11 @@ static int traverse_match(TDB_CONTEXT *ttdb, TDB_DATA kbuf, TDB_DATA dbuf,
 struct packet_struct *receive_unexpected(enum packet_type packet_type, int id,
 					 const char *mailslot_name)
 {
-	TDB_CONTEXT *tdb2;
+	struct tdb_wrap *tdb2;
 	struct receive_unexpected_state state;
 
-	tdb2 = tdb_open_log(lock_path("unexpected.tdb"), 0, 0, O_RDONLY, 0);
+	tdb2 = tdb_wrap_open(talloc_autofree_context(),
+			     lock_path("unexpected.tdb"), 0, 0, O_RDONLY, 0);
 	if (!tdb2) return NULL;
 
 	state.matched_packet = NULL;
@@ -196,9 +198,9 @@ struct packet_struct *receive_unexpected(enum packet_type packet_type, int id,
 	state.match_type = packet_type;
 	state.match_name = mailslot_name;
 
-	tdb_traverse(tdb2, traverse_match, &state);
+	tdb_traverse(tdb2->tdb, traverse_match, &state);
 
-	tdb_close(tdb2);
+	TALLOC_FREE(tdb2);
 
 	return state.matched_packet;
 }


-- 
Samba Shared Repository


More information about the samba-cvs mailing list