3 files changed, 242 insertions, 103 deletions

diff --git a/lib/talloc/talloc.3.xml b/lib/talloc/talloc.3.xml
index a327922dbe..99e8bcdb2f 100644
--- a/lib/talloc/talloc.3.xml
+++ b/lib/talloc/talloc.3.xml
@@ -783,9 +783,9 @@ if (ptr) memcpy(ptr, p, strlen(p)+1);</programlisting>
     </para>
     <para>
       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.
+      it under the terms of the GNU Lesser General Public License as 
+      published by the Free Software Foundation; either version 3 of the
+      License, or (at your option) any later version.
     </para>
     <para>
       This program is distributed in the hope that it will be useful, but
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 91452bfada..4700aa99e8 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -178,6 +178,32 @@ static struct {
 	TC_INVALIDATE_SHRINK_VALGRIND_CHUNK(_tc, _new_size); \
 } while (0)
 
+#define TC_UNDEFINE_SHRINK_FILL_CHUNK(_tc, _new_size) do { \
+	if (unlikely(talloc_fill.enabled)) { \
+		size_t _flen = (_tc)->size - (_new_size); \
+		char *_fptr = (char *)TC_PTR_FROM_CHUNK(_tc); \
+		_fptr += (_new_size); \
+		memset(_fptr, talloc_fill.fill_value, _flen); \
+	} \
+} while (0)
+
+#if defined(DEVELOPER) && defined(VALGRIND_MAKE_MEM_UNDEFINED)
+/* Mark the unused bytes as undefined */
+#define TC_UNDEFINE_SHRINK_VALGRIND_CHUNK(_tc, _new_size) do { \
+	size_t _flen = (_tc)->size - (_new_size); \
+	char *_fptr = (char *)TC_PTR_FROM_CHUNK(_tc); \
+	_fptr += (_new_size); \
+	VALGRIND_MAKE_MEM_UNDEFINED(_fptr, _flen); \
+} while (0)
+#else
+#define TC_UNDEFINE_SHRINK_VALGRIND_CHUNK(_tc, _new_size) do { } while (0)
+#endif
+
+#define TC_UNDEFINE_SHRINK_CHUNK(_tc, _new_size) do { \
+	TC_UNDEFINE_SHRINK_FILL_CHUNK(_tc, _new_size); \
+	TC_UNDEFINE_SHRINK_VALGRIND_CHUNK(_tc, _new_size); \
+} while (0)
+
 #if defined(DEVELOPER) && defined(VALGRIND_MAKE_MEM_UNDEFINED)
 /* Mark the new bytes as undefined */
 #define TC_UNDEFINE_GROW_VALGRIND_CHUNK(_tc, _new_size) do { \
@@ -683,6 +709,69 @@ _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;
+	}
+}
+
+static inline void _talloc_free_children_internal(struct talloc_chunk *tc,
+						  void *ptr,
+						  const char *location);
+
 /* 
    internal talloc_free call
 */
@@ -753,41 +842,7 @@ static inline int _talloc_free_internal(void *ptr, const char *location)
 
 	tc->flags |= TALLOC_FLAG_LOOP;
 
-	while (tc->child) {
-		/* we need to work out who will own an abandoned child
-		   if it cannot be freed. In priority order, the first
-		   choice is owner of any remaining reference to this
-		   pointer, the second choice is our parent, and the
-		   final choice is the null context. */
-		void *child = TC_PTR_FROM_CHUNK(tc->child);
-		const void *new_parent = null_context;
-		struct talloc_chunk *old_parent = NULL;
-		if (unlikely(tc->child->refs)) {
-			struct talloc_chunk *p = talloc_parent_chunk(tc->child->refs);
-			if (p) new_parent = TC_PTR_FROM_CHUNK(p);
-		}
-		/* finding the parent here is potentially quite
-		   expensive, but the alternative, which is to change
-		   talloc to always have a valid tc->parent pointer,
-		   makes realloc more expensive where there are a
-		   large number of children.
-
-		   The reason we need the parent pointer here is that
-		   if _talloc_free_internal() fails due to references
-		   or a failing destructor we need to re-parent, but
-		   the free call can invalidate the prev pointer.
-		*/
-		if (new_parent == null_context && (tc->child->refs || tc->child->destructor)) {
-			old_parent = talloc_parent_chunk(ptr);
-		}
-		if (unlikely(_talloc_free_internal(child, location) == -1)) {
-			if (new_parent == null_context) {
-				struct talloc_chunk *p = old_parent;
-				if (p) new_parent = TC_PTR_FROM_CHUNK(p);
-			}
-			_talloc_steal_internal(new_parent, child);
-		}
-	}
+	_talloc_free_children_internal(tc, ptr, location);
 
 	tc->flags |= TALLOC_FLAG_FREE;
 
@@ -797,21 +852,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!");
@@ -820,26 +864,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);
@@ -1204,21 +1234,10 @@ _PUBLIC_ void *talloc_init(const char *fmt, ...)
 	return ptr;
 }
 
-/*
-  this is a replacement for the Samba3 talloc_destroy_pool functionality. It
-  should probably not be used in new code. It's in here to keep the talloc
-  code consistent across Samba 3 and 4.
-*/
-_PUBLIC_ void talloc_free_children(void *ptr)
+static inline void _talloc_free_children_internal(struct talloc_chunk *tc,
+						  void *ptr,
+						  const char *location)
 {
-	struct talloc_chunk *tc;
-
-	if (unlikely(ptr == NULL)) {
-		return;
-	}
-
-	tc = talloc_chunk_from_ptr(ptr);
-
 	while (tc->child) {
 		/* we need to work out who will own an abandoned child
 		   if it cannot be freed. In priority order, the first
@@ -1227,13 +1246,28 @@ _PUBLIC_ void talloc_free_children(void *ptr)
 		   final choice is the null context. */
 		void *child = TC_PTR_FROM_CHUNK(tc->child);
 		const void *new_parent = null_context;
+		struct talloc_chunk *old_parent = NULL;
 		if (unlikely(tc->child->refs)) {
 			struct talloc_chunk *p = talloc_parent_chunk(tc->child->refs);
 			if (p) new_parent = TC_PTR_FROM_CHUNK(p);
 		}
-		if (unlikely(talloc_free(child) == -1)) {
+		/* finding the parent here is potentially quite
+		   expensive, but the alternative, which is to change
+		   talloc to always have a valid tc->parent pointer,
+		   makes realloc more expensive where there are a
+		   large number of children.
+
+		   The reason we need the parent pointer here is that
+		   if _talloc_free_internal() fails due to references
+		   or a failing destructor we need to re-parent, but
+		   the free call can invalidate the prev pointer.
+		*/
+		if (new_parent == null_context && (tc->child->refs || tc->child->destructor)) {
+			old_parent = talloc_parent_chunk(ptr);
+		}
+		if (unlikely(_talloc_free_internal(child, location) == -1)) {
 			if (new_parent == null_context) {
-				struct talloc_chunk *p = talloc_parent_chunk(ptr);
+				struct talloc_chunk *p = old_parent;
 				if (p) new_parent = TC_PTR_FROM_CHUNK(p);
 			}
 			_talloc_steal_internal(new_parent, child);
@@ -1241,6 +1275,24 @@ _PUBLIC_ void talloc_free_children(void *ptr)
 	}
 }
 
+/*
+  this is a replacement for the Samba3 talloc_destroy_pool functionality. It
+  should probably not be used in new code. It's in here to keep the talloc
+  code consistent across Samba 3 and 4.
+*/
+_PUBLIC_ void talloc_free_children(void *ptr)
+{
+	struct talloc_chunk *tc;
+
+	if (unlikely(ptr == NULL)) {
+		return;
+	}
+
+	tc = talloc_chunk_from_ptr(ptr);
+
+	_talloc_free_children_internal(tc, ptr, __location__);
+}
+
 /* 
    Allocate a bit of memory as a child of an existing pointer
 */
@@ -1365,7 +1417,16 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
 			}
 			return ptr;
 		} else if ((tc->size - size) < 1024) {
-			TC_INVALIDATE_SHRINK_CHUNK(tc, size);
+			/*
+			 * if we call TC_INVALIDATE_SHRINK_CHUNK() here
+			 * we would need to call TC_UNDEFINE_GROW_CHUNK()
+			 * after each realloc call, which slows down
+			 * testing a lot :-(.
+			 *
+			 * That is why we only mark memory as undefined here.
+			 */
+			TC_UNDEFINE_SHRINK_CHUNK(tc, size);
+
 			/* do not shrink if we have less than 1k to gain */
 			tc->size = size;
 			return ptr;
@@ -1410,8 +1471,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.
@@ -1438,6 +1504,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);
 
 				/*
@@ -1481,7 +1548,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);
@@ -1490,21 +1556,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 ba583ab84e..90417c6ade 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();