[SCM] Samba Shared Repository - branch master updated

Stefan Metzmacher metze at samba.org
Tue May 17 01:44:02 MDT 2011


The branch, master has been updated
       via  37b2130 talloc: test talloc_steal out of a talloc_pool
       via  16cc52c talloc: add memset() calls to test_pool()
       via  c281f2f talloc: setup the new 'tc' before TC_UNDEFINE_GROW_CHUNK()  _talloc_realloc()
       via  7102105 talloc: make really sure only optimize realloc if there's only one pool chunk
       via  14b662e talloc: make use of _talloc_free_poolmem() in _talloc_realloc()
       via  2d514be talloc: split the handling of FLAG_POOL/FLAG_POOLMEM in _talloc_free_internal
      from  28598e7 s4-dfs: Use a workaround for ndr relative pointer bug/limitation

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


- Log -----------------------------------------------------------------
commit 37b2130ed9612a7334888ecd2fee26b0b45ac271
Author: Stefan Metzmacher <metze at samba.org>
Date:   Mon May 16 19:25:47 2011 +0200

    talloc: test talloc_steal out of a talloc_pool
    
    metze
    
    Autobuild-User: Stefan Metzmacher <metze at samba.org>
    Autobuild-Date: Tue May 17 09:43:01 CEST 2011 on sn-devel-104

commit 16cc52cf70a9918843f9761baf483338c80bf1d0
Author: Stefan Metzmacher <metze at samba.org>
Date:   Tue May 17 08:20:13 2011 +0200

    talloc: add memset() calls to test_pool()
    
    This way we the pool based valgrind code.
    
    metze

commit c281f2fc1a359d0d3b91b94438f11bb7c88170b5
Author: Stefan Metzmacher <metze at samba.org>
Date:   Tue May 17 08:19:04 2011 +0200

    talloc: setup the new 'tc' before TC_UNDEFINE_GROW_CHUNK()  _talloc_realloc()
    
    metze

commit 7102105c8954627dc30a851327cf2642ac0783d5
Author: Stefan Metzmacher <metze at samba.org>
Date:   Mon May 16 20:15:59 2011 +0200

    talloc: make really sure only optimize realloc if there's only one pool chunk
    
    *talloc_pool_objectcount(pool_tc) == 2 doesn't mean the one of the objects
    is the pool itself! So we better check for == 1 and calculate the chunk count.
    
    metze

commit 14b662ee4f278764b9dfd620851e908d29f29fc4
Author: Stefan Metzmacher <metze at samba.org>
Date:   Mon May 16 20:23:13 2011 +0200

    talloc: make use of _talloc_free_poolmem() in _talloc_realloc()
    
    This should follow the same logic...
    
    metze

commit 2d514be1ed3b8245157a0a51186ec7f9db828202
Author: Stefan Metzmacher <metze at samba.org>
Date:   Mon May 16 19:06:07 2011 +0200

    talloc: split the handling of FLAG_POOL/FLAG_POOLMEM in _talloc_free_internal
    
    The optimization of the object_count == 1 case should only happen
    for when we're not destroying the pool itself. And it should only
    happen if the pool itself is still valid.
    
    If the pool isn't valid (it has TALLOC_FLAG_FREE),
    object_count == 1 does not mean that the pool is the last object,
    which can happen if you use talloc_steal/move() on memory
    from the pool and then free the pool itself.
    
    Thanks to Volker for noticing this!
    
    metze

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

Summary of changes:
 lib/talloc/talloc.c    |  122 +++++++++++++++++++++++++++++-------------------
 lib/talloc/testsuite.c |   86 ++++++++++++++++++++++++++++++++++
 2 files changed, 160 insertions(+), 48 deletions(-)


Changeset truncated at 500 lines:

diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 2a956dc..9553405 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -709,6 +709,65 @@ _PUBLIC_ void *_talloc_reference_loc(const void *context, const void *ptr, const
 
 static void *_talloc_steal_internal(const void *new_ctx, const void *ptr);
 
+static inline void _talloc_free_poolmem(struct talloc_chunk *tc,
+					const char *location)
+{
+	struct talloc_chunk *pool;
+	void *next_tc;
+	unsigned int *pool_object_count;
+
+	pool = (struct talloc_chunk *)tc->pool;
+	next_tc = TC_POOLMEM_NEXT_CHUNK(tc);
+
+	tc->flags |= TALLOC_FLAG_FREE;
+
+	/* we mark the freed memory with where we called the free
+	 * from. This means on a double free error we can report where
+	 * the first free came from
+	 */
+	tc->name = location;
+
+	TC_INVALIDATE_FULL_CHUNK(tc);
+
+	pool_object_count = talloc_pool_objectcount(pool);
+
+	if (unlikely(*pool_object_count == 0)) {
+		talloc_abort("Pool object count zero!");
+		return;
+	}
+
+	*pool_object_count -= 1;
+
+	if (unlikely(*pool_object_count == 1 && !(pool->flags & TALLOC_FLAG_FREE))) {
+		/*
+		 * if there is just one object left in the pool
+		 * and pool->flags does not have TALLOC_FLAG_FREE,
+		 * it means this is the pool itself and
+		 * the rest is available for new objects
+		 * again.
+		 */
+		pool->pool = TC_POOL_FIRST_CHUNK(pool);
+		TC_INVALIDATE_POOL(pool);
+	} else if (unlikely(*pool_object_count == 0)) {
+		/*
+		 * we mark the freed memory with where we called the free
+		 * from. This means on a double free error we can report where
+		 * the first free came from
+		 */
+		pool->name = location;
+
+		TC_INVALIDATE_FULL_CHUNK(pool);
+		free(pool);
+	} else if (pool->pool == next_tc) {
+		/*
+		 * if pool->pool still points to end of
+		 * 'tc' (which is stored in the 'next_tc' variable),
+		 * we can reclaim the memory of 'tc'.
+		 */
+		pool->pool = tc;
+	}
+}
+
 /* 
    internal talloc_free call
 */
@@ -823,21 +882,10 @@ static inline int _talloc_free_internal(void *ptr, const char *location)
 	 */	 
 	tc->name = location;
 
-	if (tc->flags & (TALLOC_FLAG_POOL|TALLOC_FLAG_POOLMEM)) {
-		struct talloc_chunk *pool;
-		void *next_tc = NULL;
+	if (tc->flags & TALLOC_FLAG_POOL) {
 		unsigned int *pool_object_count;
 
-		if (unlikely(tc->flags & TALLOC_FLAG_POOL)) {
-			pool = tc;
-		} else {
-			pool = (struct talloc_chunk *)tc->pool;
-			next_tc = TC_POOLMEM_NEXT_CHUNK(tc);
-
-			TC_INVALIDATE_FULL_CHUNK(tc);
-		}
-
-		pool_object_count = talloc_pool_objectcount(pool);
+		pool_object_count = talloc_pool_objectcount(tc);
 
 		if (unlikely(*pool_object_count == 0)) {
 			talloc_abort("Pool object count zero!");
@@ -846,26 +894,12 @@ static inline int _talloc_free_internal(void *ptr, const char *location)
 
 		*pool_object_count -= 1;
 
-		if (unlikely(*pool_object_count == 1)) {
-			/*
-			 * if there is just object left in the pool
-			 * it means this is the pool itself and
-			 * the rest is available for new objects
-			 * again.
-			 */
-			pool->pool = TC_POOL_FIRST_CHUNK(pool);
-			TC_INVALIDATE_POOL(pool);
-		} else if (unlikely(*pool_object_count == 0)) {
-			TC_INVALIDATE_FULL_CHUNK(pool);
-			free(pool);
-		} else if (pool->pool == next_tc) {
-			/*
-			 * if pool->pool still points to end of
-			 * 'tc' (which is stored in the 'next_tc' variable),
-			 * we can reclaim the memory of 'tc'.
-			 */
-			pool->pool = tc;
+		if (unlikely(*pool_object_count == 0)) {
+			TC_INVALIDATE_FULL_CHUNK(tc);
+			free(tc);
 		}
+	} else if (tc->flags & TALLOC_FLAG_POOLMEM) {
+		_talloc_free_poolmem(tc, location);
 	} else {
 		TC_INVALIDATE_FULL_CHUNK(tc);
 		free(tc);
@@ -1445,8 +1479,13 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
 		size_t new_chunk_size = TC_ALIGN16(TC_HDR_SIZE + size);
 		size_t space_needed;
 		size_t space_left;
+		unsigned int chunk_count = *talloc_pool_objectcount(pool_tc);
+
+		if (!(pool_tc->flags & TALLOC_FLAG_FREE)) {
+			chunk_count -= 1;
+		}
 
-		if (*talloc_pool_objectcount(pool_tc) == 2) {
+		if (chunk_count == 1) {
 			/*
 			 * optimize for the case where 'tc' is the only
 			 * chunk in the pool.
@@ -1473,6 +1512,7 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
 				memmove(pool_tc->pool, tc, old_used);
 				new_ptr = pool_tc->pool;
 
+				tc = (struct talloc_chunk *)new_ptr;
 				TC_UNDEFINE_GROW_CHUNK(tc, size);
 
 				/*
@@ -1516,7 +1556,6 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
 		}
 
 		new_ptr = talloc_alloc_pool(tc, size + TC_HDR_SIZE);
-		*talloc_pool_objectcount(pool_tc) -= 1;
 
 		if (new_ptr == NULL) {
 			new_ptr = malloc(TC_HDR_SIZE+size);
@@ -1525,21 +1564,8 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
 
 		if (new_ptr) {
 			memcpy(new_ptr, tc, MIN(tc->size,size) + TC_HDR_SIZE);
-			TC_INVALIDATE_FULL_CHUNK(tc);
 
-			if (*talloc_pool_objectcount(pool_tc) == 1) {
-				/*
-				 * If the pool is empty now reclaim everything.
-				 */
-				pool_tc->pool = TC_POOL_FIRST_CHUNK(pool_tc);
-				TC_INVALIDATE_POOL(pool_tc);
-			} else if (next_tc == pool_tc->pool) {
-				/*
-				 * If it was reallocated and tc was the last
-				 * chunk, we can reclaim the memory of tc.
-				 */
-				pool_tc->pool = tc;
-			}
+			_talloc_free_poolmem(tc, __location__ "_talloc_realloc");
 		}
 	}
 	else {
diff --git a/lib/talloc/testsuite.c b/lib/talloc/testsuite.c
index ba583ab..90417c6 100644
--- a/lib/talloc/testsuite.c
+++ b/lib/talloc/testsuite.c
@@ -1128,23 +1128,31 @@ static bool test_pool(void)
 	pool = talloc_pool(NULL, 1024);
 
 	p1 = talloc_size(pool, 80);
+	memset(p1, 0x11, talloc_get_size(p1));
 	p2 = talloc_size(pool, 20);
+	memset(p2, 0x11, talloc_get_size(p2));
 	p3 = talloc_size(p1, 50);
+	memset(p3, 0x11, talloc_get_size(p3));
 	p4 = talloc_size(p3, 1000);
+	memset(p4, 0x11, talloc_get_size(p4));
 
 #if 1 /* this relies on ALWAYS_REALLOC == 0 in talloc.c */
 	p2_2 = talloc_realloc_size(pool, p2, 20+1);
 	torture_assert("pool realloc 20+1", p2_2 == p2, "failed: pointer changed");
+	memset(p2, 0x11, talloc_get_size(p2));
 	p2_2 = talloc_realloc_size(pool, p2, 20-1);
 	torture_assert("pool realloc 20-1", p2_2 == p2, "failed: pointer changed");
+	memset(p2, 0x11, talloc_get_size(p2));
 	p2_2 = talloc_realloc_size(pool, p2, 20-1);
 	torture_assert("pool realloc 20-1", p2_2 == p2, "failed: pointer changed");
+	memset(p2, 0x11, talloc_get_size(p2));
 
 	talloc_free(p3);
 
 	/* this should reclaim the memory of p4 and p3 */
 	p2_2 = talloc_realloc_size(pool, p2, 400);
 	torture_assert("pool realloc 400", p2_2 == p2, "failed: pointer changed");
+	memset(p2, 0x11, talloc_get_size(p2));
 
 	talloc_free(p1);
 
@@ -1152,37 +1160,46 @@ static bool test_pool(void)
 	p2_2 = talloc_realloc_size(pool, p2, 800);
 	torture_assert("pool realloc 800", p2_2 == p1, "failed: pointer not changed");
 	p2 = p2_2;
+	memset(p2, 0x11, talloc_get_size(p2));
 
 	/* this should do a malloc */
 	p2_2 = talloc_realloc_size(pool, p2, 1800);
 	torture_assert("pool realloc 1800", p2_2 != p2, "failed: pointer not changed");
 	p2 = p2_2;
+	memset(p2, 0x11, talloc_get_size(p2));
 
 	/* this should reclaim the memory from the pool */
 	p3 = talloc_size(pool, 80);
 	torture_assert("pool alloc 80", p3 == p1, "failed: pointer changed");
+	memset(p3, 0x11, talloc_get_size(p3));
 
 	talloc_free(p2);
 	talloc_free(p3);
 
 	p1 = talloc_size(pool, 80);
+	memset(p1, 0x11, talloc_get_size(p1));
 	p2 = talloc_size(pool, 20);
+	memset(p2, 0x11, talloc_get_size(p2));
 
 	talloc_free(p1);
 
 	p2_2 = talloc_realloc_size(pool, p2, 20-1);
 	torture_assert("pool realloc 20-1", p2_2 == p2, "failed: pointer changed");
+	memset(p2, 0x11, talloc_get_size(p2));
 	p2_2 = talloc_realloc_size(pool, p2, 20-1);
 	torture_assert("pool realloc 20-1", p2_2 == p2, "failed: pointer changed");
+	memset(p2, 0x11, talloc_get_size(p2));
 
 	/* this should do a malloc */
 	p2_2 = talloc_realloc_size(pool, p2, 1800);
 	torture_assert("pool realloc 1800", p2_2 != p2, "failed: pointer not changed");
 	p2 = p2_2;
+	memset(p2, 0x11, talloc_get_size(p2));
 
 	/* this should reclaim the memory from the pool */
 	p3 = talloc_size(pool, 800);
 	torture_assert("pool alloc 800", p3 == p1, "failed: pointer changed");
+	memset(p3, 0x11, talloc_get_size(p3));
 
 #endif /* this relies on ALWAYS_REALLOC == 0 in talloc.c */
 
@@ -1191,6 +1208,73 @@ static bool test_pool(void)
 	return true;
 }
 
+static bool test_pool_steal(void)
+{
+	void *root;
+	void *pool;
+	void *p1, *p2;
+	void *p1_2, *p2_2;
+	size_t hdr;
+	size_t ofs1, ofs2;
+
+	root = talloc_new(NULL);
+	pool = talloc_pool(root, 1024);
+
+	p1 = talloc_size(pool, 4 * 16);
+	torture_assert("pool allocate 4 * 16", p1 != NULL, "failed ");
+	memset(p1, 0x11, talloc_get_size(p1));
+	p2 = talloc_size(pool, 4 * 16);
+	torture_assert("pool allocate 4 * 16", p2 > p1, "failed: !(p2 > p1) ");
+	memset(p2, 0x11, talloc_get_size(p2));
+
+	ofs1 = PTR_DIFF(p2, p1);
+	hdr = ofs1 - talloc_get_size(p1);
+
+	talloc_steal(root, p1);
+	talloc_steal(root, p2);
+
+	talloc_free(pool);
+
+	p1_2 = p1;
+
+#if 1 /* this relies on ALWAYS_REALLOC == 0 in talloc.c */
+	p1_2 = talloc_realloc_size(root, p1, 5 * 16);
+	torture_assert("pool realloc 5 * 16", p1_2 > p2, "failed: pointer not changed");
+	memset(p1_2, 0x11, talloc_get_size(p1_2));
+	ofs1 = PTR_DIFF(p1_2, p2);
+	ofs2 = talloc_get_size(p2) + hdr;
+
+	torture_assert("pool realloc ", ofs1 == ofs2, "failed: pointer offset unexpected");
+
+	p2_2 = talloc_realloc_size(root, p2, 3 * 16);
+	torture_assert("pool realloc 5 * 16", p2_2 == p2, "failed: pointer changed");
+	memset(p2_2, 0x11, talloc_get_size(p2_2));
+#endif /* this relies on ALWAYS_REALLOC == 0 in talloc.c */
+
+	talloc_free(p1_2);
+
+	p2_2 = p2;
+
+#if 1 /* this relies on ALWAYS_REALLOC == 0 in talloc.c */
+	/* now we should reclaim the full pool */
+	p2_2 = talloc_realloc_size(root, p2, 8 * 16);
+	torture_assert("pool realloc 8 * 16", p2_2 == p1, "failed: pointer not expected");
+	p2 = p2_2;
+	memset(p2_2, 0x11, talloc_get_size(p2_2));
+
+	/* now we malloc and free the full pool space */
+	p2_2 = talloc_realloc_size(root, p2, 2 * 1024);
+	torture_assert("pool realloc 2 * 1024", p2_2 != p1, "failed: pointer not expected");
+	memset(p2_2, 0x11, talloc_get_size(p2_2));
+
+#endif /* this relies on ALWAYS_REALLOC == 0 in talloc.c */
+
+	talloc_free(p2_2);
+
+	talloc_free(root);
+
+	return true;
+}
 
 static bool test_free_ref_null_context(void)
 {
@@ -1290,6 +1374,8 @@ bool torture_local_talloc(struct torture_context *tctx)
 	test_reset();
 	ret &= test_pool();
 	test_reset();
+	ret &= test_pool_steal();
+	test_reset();
 	ret &= test_free_ref_null_context();
 	test_reset();
 	ret &= test_rusty();


-- 
Samba Shared Repository


More information about the samba-cvs mailing list