[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Fri Feb 26 21:36:05 UTC 2016


The branch, master has been updated
       via  bba426d smbXsrv_open: factor fetch-locking of local record into function
       via  7b8cfa1 smbXsrv_open: factor fetch-locking of global record into function
       via  f3ea162 lib/util: Add tests for strv
       via  0d08af7 lib/util/idtree.c: fix UB by using uint in left-shift
      from  39081af selftest: Add a blackbox test for smbget

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


- Log -----------------------------------------------------------------
commit bba426dbbbe0e4aebd99b78cff037eac754aa282
Author: Michael Adam <obnox at samba.org>
Date:   Fri Feb 26 00:53:22 2016 +0100

    smbXsrv_open: factor fetch-locking of local record into function
    
    smbXsrv_open_local_fetch_locked()
    
    Signed-off-by: Michael Adam <obnox at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Fri Feb 26 22:35:28 CET 2016 on sn-devel-144

commit 7b8cfa1cbc2738f798fad748ea40c029d99814a0
Author: Michael Adam <obnox at samba.org>
Date:   Fri Feb 26 00:41:24 2016 +0100

    smbXsrv_open: factor fetch-locking of global record into function
    
    smbXsrv_open_global_fetch_locked()
    
    Signed-off-by: Michael Adam <obnox at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit f3ea162488c52a9b7ca82b191007df3ba8185c87
Author: Martin Schwenke <martin at meltin.net>
Date:   Fri Feb 26 15:23:31 2016 +1100

    lib/util: Add tests for strv
    
    These are blackbox tests against most of the API.
    
    It would be possible to write tests that check the internals of the
    strv are as expected but that probably doesn't add much value.
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit 0d08af70c83f98bf0daeef45a0d76676a1fbde5a
Author: Aurelien Aptel <aaptel at suse.com>
Date:   Thu Feb 25 13:58:01 2016 +0100

    lib/util/idtree.c: fix UB by using uint in left-shift
    
    Using negative values on the left-side of a left-shift operation is an
    Undefined Behaviour in C.
    
    Signed-off-by: Aurelien Aptel <aaptel at suse.com>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ira Cooper <ira at samba.org>

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

Summary of changes:
 lib/util/idtree.c                   |   2 +-
 lib/util/tests/strv.c               | 169 ++++++++++++++++++++++++++++++++++++
 source3/smbd/smbXsrv_open.c         | 133 +++++++++++++---------------
 source4/torture/local/local.c       |   1 +
 source4/torture/local/wscript_build |   1 +
 5 files changed, 234 insertions(+), 72 deletions(-)
 create mode 100644 lib/util/tests/strv.c


Changeset truncated at 500 lines:

diff --git a/lib/util/idtree.c b/lib/util/idtree.c
index e2cfcc5..72266a6 100644
--- a/lib/util/idtree.c
+++ b/lib/util/idtree.c
@@ -289,7 +289,7 @@ static void *_idr_find(struct idr_context *idp, int id)
 	 * present.  If so, tain't one of ours!
 	 */
 	if (n + IDR_BITS < 31 &&
-	    ((id & ~(~0 << MAX_ID_SHIFT)) >> (n + IDR_BITS))) {
+	    ((id & ~(~0U << MAX_ID_SHIFT)) >> (n + IDR_BITS))) {
 		return NULL;
 	}
 
diff --git a/lib/util/tests/strv.c b/lib/util/tests/strv.c
new file mode 100644
index 0000000..4030c44
--- /dev/null
+++ b/lib/util/tests/strv.c
@@ -0,0 +1,169 @@
+/*
+ * Tests for strv
+ *
+ * Copyright Martin Schwenke <martin at meltin.net> 2016
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+
+#include <talloc.h>
+
+#include "replace.h"
+
+#include "libcli/util/ntstatus.h"
+#include "torture/torture.h"
+#include "lib/util/data_blob.h"
+#include "torture/local/proto.h"
+
+#include "lib/util/strv.h"
+
+static bool test_strv_empty(struct torture_context *tctx)
+{
+	/* NULL strv contains 0 entries */
+	torture_assert_int_equal(tctx,
+				 strv_count(NULL),
+				 0,
+				 "strv_count() on NULL failed");
+
+	/* NULL strv has no next entry */
+	torture_assert(tctx,
+		       strv_next(NULL, NULL) == NULL,
+		       "strv_next() on NULL failed");
+
+	return true;
+}
+
+static bool test_strv_single(struct torture_context *tctx)
+{
+	const char *data = "foo";
+	char *strv = NULL;
+	char *t;
+	int ret;
+
+	/* Add an item */
+	ret = strv_add(tctx, &strv, data);
+	torture_assert(tctx, ret == 0, "strv_add() failed");
+
+	/* Is there 1 item? */
+	torture_assert_int_equal(tctx,
+				 strv_count(strv), 1,
+				 "strv_count() failed");
+
+	/* Is the expected item the first one? */
+	t = strv_next(strv, NULL);
+	torture_assert(tctx,
+		       strcmp(t, data) == 0,
+		       "strv_next() failed");
+
+	/* Can the expected item be found? */
+	t = strv_find(strv, data);
+	torture_assert(tctx,
+		       strcmp(t, data) == 0,
+		       "strv_next() failed");
+
+	/* Delete it */
+	strv_delete(&strv, t);
+
+	/* Should have no items */
+	torture_assert_int_equal(tctx,
+				 strv_count(strv), 0,
+				 "strv_count() failed");
+	return true;
+}
+
+static bool test_strv_multi(struct torture_context *tctx)
+{
+	const char *data[5] = { "foo", "bar", "", "samba", "x"};
+	char *strv = NULL;
+	char *t;
+	int i, ret;
+	const int num = sizeof(data) / sizeof(data[0]);
+
+	/* Add items */
+	for (i = 0; i < num; i++) {
+		ret = strv_add(tctx, &strv, data[i]);
+		torture_assert(tctx, ret == 0, "strv_add() failed");
+	}
+
+	torture_assert_int_equal(tctx,
+				 strv_count(strv), num,
+				 "strv_count() failed");
+
+	/* Check that strv_next() finds the expected values */
+	t = NULL;
+	for (i = 0; i < num; i++) {
+		t = strv_next(strv, t);
+		torture_assert(tctx,
+			       strcmp(t, data[i]) == 0,
+			       "strv_next() failed");
+	}
+
+
+	/* Check that strv_next() finds the expected values */
+	t = NULL;
+	for (i = 0; i < num; i++) {
+		t = strv_next(strv, t);
+		torture_assert(tctx,
+			       strcmp(t, data[i]) == 0,
+			       "strv_next() failed");
+	}
+
+	/* Find each item, delete it, check count */
+	for (i = 0; i < num; i++) {
+		t = strv_find(strv, data[i]);
+		torture_assert(tctx,
+			       strcmp(t, data[i]) == 0,
+			       "strv_next() failed");
+		strv_delete(&strv, t);
+		torture_assert_int_equal(tctx,
+					 strv_count(strv), num - i - 1,
+					 "strv_delete() failed");
+	}
+
+	/* Add items */
+	for (i = 0; i < num; i++) {
+		ret = strv_add(tctx, &strv, data[i]);
+		torture_assert(tctx, ret == 0, "strv_add() failed");
+	}
+
+	torture_assert_int_equal(tctx,
+				 strv_count(strv), num,
+				 "strv_count() failed");
+
+	/* Find items in reverse, delete, check count */
+	for (i = num - 1; i >= 0; i--) {
+		t = strv_find(strv, data[i]);
+		torture_assert(tctx,
+			       strcmp(t, data[i]) == 0,
+			       "strv_next() failed");
+		strv_delete(&strv, t);
+		torture_assert_int_equal(tctx,
+					 strv_count(strv), i,
+					 "strv_delete() failed");
+	}
+
+	return true;
+}
+
+struct torture_suite *torture_local_util_strv(TALLOC_CTX *mem_ctx)
+{
+	struct torture_suite *suite = torture_suite_create(mem_ctx, "strv");
+
+	torture_suite_add_simple_test(suite, "strv_empty",  test_strv_empty);
+	torture_suite_add_simple_test(suite, "strv_single", test_strv_single);
+	torture_suite_add_simple_test(suite, "strv_multi",  test_strv_multi);
+
+	return suite;
+}
diff --git a/source3/smbd/smbXsrv_open.c b/source3/smbd/smbXsrv_open.c
index 1fe8b1b..ee35f2d 100644
--- a/source3/smbd/smbXsrv_open.c
+++ b/source3/smbd/smbXsrv_open.c
@@ -151,6 +151,48 @@ 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;
+
+	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 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;
+
+	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,
@@ -275,8 +317,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;
 
@@ -290,9 +330,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;
 		}
@@ -342,16 +380,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;
 		}
@@ -494,8 +528,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;
@@ -509,9 +541,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;
@@ -717,8 +747,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;
 
@@ -728,15 +756,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;
 	}
 
@@ -909,8 +932,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): "
@@ -919,17 +940,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;
 	}
 
@@ -979,21 +994,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;
 		}
 	}
@@ -1052,19 +1057,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;
 		}
 	}
@@ -1403,21 +1399,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;
 	}
 
diff --git a/source4/torture/local/local.c b/source4/torture/local/local.c
index 3988988..1f68aac 100644
--- a/source4/torture/local/local.c
+++ b/source4/torture/local/local.c
@@ -41,6 +41,7 @@
 	torture_local_util_data_blob, 
 	torture_local_util_asn1,
 	torture_local_util_anonymous_shared,
+	torture_local_util_strv,
 	torture_local_idtree, 
 	torture_local_dlinklist,
 	torture_local_genrand, 
diff --git a/source4/torture/local/wscript_build b/source4/torture/local/wscript_build
index eb45df8..ef1fb30 100644
--- a/source4/torture/local/wscript_build
+++ b/source4/torture/local/wscript_build
@@ -17,6 +17,7 @@ TORTURE_LOCAL_SOURCE = '''../../../lib/util/charset/tests/iconv.c
 	dbspeed.c torture.c ../ldb/ldb.c ../../dsdb/common/tests/dsdb_dn.c
 	../../dsdb/schema/tests/schema_syntax.c
 	../../../lib/util/tests/anonymous_shared.c
+	../../../lib/util/tests/strv.c
 	verif_trailer.c
 	nss_tests.c
 	fsrvp_state.c'''


-- 
Samba Shared Repository



More information about the samba-cvs mailing list