[SCM] Samba Shared Repository - branch v3-6-test updated

Karolin Seeger kseeger at samba.org
Sat Aug 20 12:56:46 MDT 2011


The branch, v3-6-test has been updated
       via  8d46b29 talloc: check block count aftter references test
       via  2e07c66 talloc: added test suite for talloc_free_children()
       via  39b7c96 Fix license info for talloc in manpage.
       via  0c5a41d talloc - improve doxygen comment of "talloc_move"
       via  db9670d talloc - some documentation changes
       via  99495ea talloc: preserve context name on talloc_free_children()
       via  0755408 talloc: ensure the sibling linked list remains valid during a free
      from  909ff85 s3-vfs: Fix vfs_chown_fsp.

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-6-test


- Log -----------------------------------------------------------------
commit 8d46b29595c4da27b175b25a08b945fbeeeab3ec
Author: Andrew Tridgell <tridge at samba.org>
Date:   Thu Aug 4 12:07:19 2011 +1000

    talloc: check block count aftter references test
    
    Pair-Programmed-With: Amitay Isaacs <amitay at gmail.com>
    (cherry picked from commit 73677875b46251f59b66c9713f1decc89bd2ea3e)
    
    The last 7 patches address bug #8384 (Windows XP clients seem to crash smbd
    process every once in a while).

commit 2e07c662b08f8bba5cc03efcdcc13165e392b6e8
Author: Andrew Tridgell <tridge at samba.org>
Date:   Fri Jul 29 11:57:07 2011 +1000

    talloc: added test suite for talloc_free_children()
    
    this tests the fix from Simo
    
    Autobuild-User: Andrew Tridgell <tridge at samba.org>
    Autobuild-Date: Fri Jul 29 11:30:13 CEST 2011 on sn-devel-104
    (cherry picked from commit d004fd0b53fb6f3ae64f0e24cf51f4471d434574)

commit 39b7c96517995d2b14e19a006cb18ef642cb98ff
Author: Jelmer Vernooij <jelmer at samba.org>
Date:   Sun Apr 24 02:39:14 2011 +0200

    Fix license info for talloc in manpage.
    
    Autobuild-User: Jelmer Vernooij <jelmer at samba.org>
    Autobuild-Date: Sun Apr 24 03:27:54 CEST 2011 on sn-devel-104
    (cherry picked from commit fb05e82c99f0779bd44371a2bdafdd7147448dd5)

commit 0c5a41da1375bab225274833ba149ba47df42aec
Author: Matthias Dieter Wallnöfer <mdw at samba.org>
Date:   Thu Mar 24 09:41:19 2011 +0100

    talloc - improve doxygen comment of "talloc_move"
    
    Express better that this should be a pointer of a pointer.
    
    Reviewed-by: Tridge
    (cherry picked from commit 6723e90d524c8e73f19c06b3ff28867a1a89b14b)

commit db9670dd163432d6758a81c5aa6000c13e61b14a
Author: Matthias Dieter Wallnöfer <mdw at samba.org>
Date:   Thu Mar 31 21:32:51 2011 +0200

    talloc - some documentation changes
    
    - Fix some typos
    - Document better the differences in the behaviour between talloc 1.X and 2.X.
      Previously this seemed a bit spongy to me.
    
    Reviewed-by: Jelmer + Tridge
    
    Autobuild-User: Matthias Dieter Wallnöfer <mdw at samba.org>
    Autobuild-Date: Mon Apr  4 11:05:42 CEST 2011 on sn-devel-104
    (cherry picked from commit 513574afd759bcb3832374d3ae170f1ed0b0062b)

commit 99495ea4d28de530fedc11c94d36bc304cf9ae4a
Author: Simo Sorce <idra at samba.org>
Date:   Wed Jul 27 12:02:35 2011 -0400

    talloc: preserve context name on talloc_free_children()
    
    Otherwise tc->name will end up pointing to garbage when it is not
    set to a const but rather to a string allocate as child of the context itself.
    
    Signed-off-by: Andrew Tridgell <tridge at samba.org>
    (cherry picked from commit 52182a528117c4dd9624f64b34a873c0903ad70a)

commit 07554082cc9d286ca0628179c9e7f7a493016a57
Author: Andrew Tridgell <tridge at samba.org>
Date:   Mon Aug 8 18:24:32 2011 +1000

    talloc: ensure the sibling linked list remains valid during a free
    
    This ensures that the sibling list of a pointer doesn't become invalid
    during a free operation. It is an alternative fix to the fix in
    6f51a1f45bf4de062cce7a562477e8140630a53d, and avoids the problem of
    trying to calculate the parent pointer early
    
    This should fix the subtle spoolss talloc bug that Simo found
    
    Autobuild-User: Andrew Tridgell <tridge at samba.org>
    Autobuild-Date: Tue Aug  9 01:53:17 CEST 2011 on sn-devel-104
    (cherry picked from commit cf986f200804ce873b43c1ecf2d5e1bd08eb8a25)

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

Summary of changes:
 lib/talloc/talloc.3.xml     |    6 ++--
 lib/talloc/talloc.c         |   41 ++++++++++++++++++++-----------
 lib/talloc/talloc.h         |   55 +++++++++++++++++++++----------------------
 lib/talloc/talloc_guide.txt |   52 ++++++++++++++++++----------------------
 lib/talloc/testsuite.c      |   45 +++++++++++++++++++++++++++++++++++
 5 files changed, 125 insertions(+), 74 deletions(-)


Changeset truncated at 500 lines:

diff --git a/lib/talloc/talloc.3.xml b/lib/talloc/talloc.3.xml
index a327922..bfb2041 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 4700aa9..19e6a37 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -838,6 +838,7 @@ static inline int _talloc_free_internal(void *ptr, const char *location)
 	} else {
 		if (tc->prev) tc->prev->next = tc->next;
 		if (tc->next) tc->next->prev = tc->prev;
+		tc->prev = tc->next = NULL;
 	}
 
 	tc->flags |= TALLOC_FLAG_LOOP;
@@ -925,6 +926,7 @@ static void *_talloc_steal_internal(const void *new_ctx, const void *ptr)
 	} else {
 		if (tc->prev) tc->prev->next = tc->next;
 		if (tc->next) tc->next->prev = tc->prev;
+		tc->prev = tc->next = NULL;
 	}
 
 	tc->parent = new_tc;
@@ -1251,23 +1253,9 @@ static inline void _talloc_free_children_internal(struct talloc_chunk *tc,
 			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;
+				struct talloc_chunk *p = talloc_parent_chunk(ptr);
 				if (p) new_parent = TC_PTR_FROM_CHUNK(p);
 			}
 			_talloc_steal_internal(new_parent, child);
@@ -1282,6 +1270,7 @@ static inline void _talloc_free_children_internal(struct talloc_chunk *tc,
 */
 _PUBLIC_ void talloc_free_children(void *ptr)
 {
+	struct talloc_chunk *tc_name = NULL;
 	struct talloc_chunk *tc;
 
 	if (unlikely(ptr == NULL)) {
@@ -1290,7 +1279,29 @@ _PUBLIC_ void talloc_free_children(void *ptr)
 
 	tc = talloc_chunk_from_ptr(ptr);
 
+	/* we do not want to free the context name if it is a child .. */
+	if (likely(tc->child)) {
+		for (tc_name = tc->child; tc_name; tc_name = tc_name->next) {
+			if (tc->name == TC_PTR_FROM_CHUNK(tc_name)) break;
+		}
+		if (tc_name) {
+			_TLIST_REMOVE(tc->child, tc_name);
+			if (tc->child) {
+				tc->child->parent = tc;
+			}
+		}
+	}
+
 	_talloc_free_children_internal(tc, ptr, __location__);
+
+	/* .. so we put it back after all other children have been freed */
+	if (tc_name) {
+		if (tc->child) {
+			tc->child->parent = NULL;
+		}
+		tc_name->parent = tc;
+		_TLIST_ADD(tc->child, tc_name);
+	}
 }
 
 /* 
diff --git a/lib/talloc/talloc.h b/lib/talloc/talloc.h
index 5710861..96c7e24 100644
--- a/lib/talloc/talloc.h
+++ b/lib/talloc/talloc.h
@@ -173,18 +173,11 @@ void *talloc_init(const char *fmt, ...) PRINTF_ATTRIBUTE(1,2);
  * destructors. Likewise, if "ptr" is NULL, then the function will make
  * no modifications and return -1.
  *
- * If this pointer has an additional parent when talloc_free() is called
- * then the memory is not actually released, but instead the most
- * recently established parent is destroyed. See talloc_reference() for
- * details on establishing additional parents.
- *
- * For more control on which parent is removed, see talloc_unlink()
- *
- * talloc_free() operates recursively on its children.
- *
- * From the 2.0 version of talloc, as a special case, talloc_free() is
- * refused on pointers that have more than one parent, as talloc would
- * have no way of knowing which parent should be removed. To free a
+ * From version 2.0 and onwards, as a special case, talloc_free() is
+ * refused on pointers that have more than one parent associated, as talloc
+ * would have no way of knowing which parent should be removed. This is
+ * different from older versions in the sense that always the reference to
+ * the most recently established parent has been destroyed. Hence to free a
  * pointer that has more than one parent please use talloc_unlink().
  *
  * To help you find problems in your code caused by this behaviour, if
@@ -201,6 +194,8 @@ void *talloc_init(const char *fmt, ...) PRINTF_ATTRIBUTE(1,2);
  * talloc_set_log_stderr() for more information on talloc logging
  * functions.
  *
+ * talloc_free() operates recursively on its children.
+ *
  * @param[in]  ptr      The chunk to be freed.
  *
  * @return              Returns 0 on success and -1 on error. A possible
@@ -233,7 +228,10 @@ int _talloc_free(void *ptr, const char *location);
  * The function walks along the list of all children of a talloc context and
  * talloc_free()s only the children, not the context itself.
  *
- * @param[in]  ptr      The chunk that you want to free the children of.
+ * A NULL argument is handled as no-op.
+ *
+ * @param[in]  ptr      The chunk that you want to free the children of
+ *                      (NULL is allowed too)
  */
 void talloc_free_children(void *ptr);
 
@@ -402,14 +400,14 @@ const char *talloc_set_name(const void *ptr, const char *fmt, ...) PRINTF_ATTRIB
  *
  * @param[in]  new_ctx  The new parent context.
  *
- * @param[in]  ptr      Pointer to the talloc chunk to move.
+ * @param[in]  pptr     Pointer to the talloc chunk to move.
  *
  * @return              The pointer of the talloc chunk it has been moved to,
  *                      NULL on error.
  */
-void *talloc_move(const void *new_ctx, const void *ptr);
+void *talloc_move(const void *new_ctx, void **pptr);
 #else
-#define talloc_move(ctx, ptr) (_TALLOC_TYPEOF(*(ptr)))_talloc_move((ctx),(void *)(ptr))
+#define talloc_move(ctx, pptr) (_TALLOC_TYPEOF(*(pptr)))_talloc_move((ctx),(void *)(pptr))
 void *_talloc_move(const void *new_ctx, const void *pptr);
 #endif
 
@@ -702,9 +700,9 @@ void *_talloc_memdup(const void *t, const void *p, size_t size, const char *name
 /**
  * @brief Assign a type to a talloc chunk.
  *
- * This macro allows you to force the name of a pointer to be a particular type.
- * This can be used in conjunction with talloc_get_type() to do type checking on
- * void* pointers.
+ * This macro allows you to force the name of a pointer to be of a particular
+ * type. This can be used in conjunction with talloc_get_type() to do type
+ * checking on void* pointers.
  *
  * It is equivalent to this:
  *
@@ -905,9 +903,9 @@ size_t talloc_reference_count(const void *ptr);
  *   will reduce the number of parents of this pointer by 1, and will
  *   cause this pointer to be freed if it runs out of parents.
  *
- * - you can talloc_free() the pointer itself. That will destroy the
- *   most recently established parent to the pointer and leave the
- *   pointer as a child of its current parent.
+ * - you can talloc_free() the pointer itself if it has at maximum one
+ *   parent. This behaviour has been changed since the release of version
+ *   2.0. Further informations in the description of "talloc_free".
  *
  * For more control on which parent to remove, see talloc_unlink()
  * @param[in]  ctx      The additional parent.
@@ -942,9 +940,10 @@ void *_talloc_reference_loc(const void *context, const void *ptr, const char *lo
  * either be a context used in talloc_reference() with this pointer, or must be
  * a direct parent of ptr.
  *
- * Usually you can just use talloc_free() instead of talloc_unlink(), but
- * sometimes it is useful to have the additional control on which parent is
- * removed.
+ * You can just use talloc_free() instead of talloc_unlink() if there
+ * is at maximum one parent. This behaviour has been changed since the
+ * release of version 2.0. Further informations in the description of
+ * "talloc_free".
  *
  * @param[in]  context  The talloc parent to remove.
  *
@@ -992,7 +991,7 @@ void *talloc_autofree_context(void);
 /**
  * @brief Get the size of a talloc chunk.
  *
- * This function lets you know the amount of memory alloced so far by
+ * This function lets you know the amount of memory allocated so far by
  * this context. It does NOT account for subcontext memory.
  * This can be used to calculate the size of an array.
  *
@@ -1453,7 +1452,7 @@ char *talloc_asprintf(const void *t, const char *fmt, ...) PRINTF_ATTRIBUTE(2,3)
  * @brief Append a formatted string to another string.
  *
  * This function appends the given formatted string to the given string. Use
- * this varient when the string in the current talloc buffer may have been
+ * this variant when the string in the current talloc buffer may have been
  * truncated in length.
  *
  * This functions sets the name of the new pointer to the new
@@ -1551,7 +1550,7 @@ void talloc_report_depth_file(const void *ptr, int depth, int max_depth, FILE *f
  * @brief Print a summary report of all memory used by ptr.
  *
  * This provides a more detailed report than talloc_report(). It will
- * recursively print the ensire tree of memory referenced by the
+ * recursively print the entire tree of memory referenced by the
  * pointer. References in the tree are shown by giving the name of the
  * pointer that is referenced.
  *
diff --git a/lib/talloc/talloc_guide.txt b/lib/talloc/talloc_guide.txt
index f29b1d6..16afc9b 100644
--- a/lib/talloc/talloc_guide.txt
+++ b/lib/talloc/talloc_guide.txt
@@ -26,7 +26,7 @@ you can do this::
   X->name = talloc_strdup(X, "foo");
 
 and the pointer X->name would be a "child" of the talloc context "X"
-which is itself a child of mem_ctx. So if you do talloc_free(mem_ctx)
+which is itself a child of "mem_ctx". So if you do talloc_free(mem_ctx)
 then it is all destroyed, whereas if you do talloc_free(X) then just X
 and X->name are destroyed, and if you do talloc_free(X->name) then
 just the name element of X is destroyed.
@@ -64,7 +64,7 @@ Multi-threading
 talloc itself does not deal with threads. It is thread-safe (assuming  
 the underlying "malloc" is), as long as each thread uses different  
 memory contexts.
-If two threads uses the same context then they need to synchronize in  
+If two threads use the same context then they need to synchronize in
 order to be safe. In particular:
 - when using talloc_enable_leak_report(), giving directly NULL as a  
 parent context implicitly refers to a hidden "null context" global  
@@ -136,18 +136,11 @@ returned -1. See talloc_set_destructor() for details on
 destructors. Likewise, if "ptr" is NULL, then the function will make
 no modifications and returns -1.
 
-If this pointer has an additional parent when talloc_free() is called
-then the memory is not actually released, but instead the most
-recently established parent is destroyed. See talloc_reference() for
-details on establishing additional parents.
-
-For more control on which parent is removed, see talloc_unlink()
-
-talloc_free() operates recursively on its children.
-
-From the 2.0 version of talloc, as a special case, talloc_free() is
-refused on pointers that have more than one parent, as talloc would
-have no way of knowing which parent should be removed. To free a
+From version 2.0 and onwards, as a special case, talloc_free() is
+refused on pointers that have more than one parent associated, as talloc
+would have no way of knowing which parent should be removed. This is
+different from older versions in the sense that always the reference to
+the most recently established parent has been destroyed. Hence to free a
 pointer that has more than one parent please use talloc_unlink().
 
 To help you find problems in your code caused by this behaviour, if
@@ -162,13 +155,16 @@ Please see the documentation for talloc_set_log_fn() and
 talloc_set_log_stderr() for more information on talloc logging
 functions.
 
+talloc_free() operates recursively on its children.
+
 =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
-int talloc_free_children(void *ptr);
+void talloc_free_children(void *ptr);
 
 The talloc_free_children() walks along the list of all children of a
 talloc context and talloc_free()s only the children, not the context
 itself.
 
+A NULL argument is handled as no-op.
 
 =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 void *talloc_reference(const void *context, const void *ptr);
@@ -190,9 +186,9 @@ ways:
     will reduce the number of parents of this pointer by 1, and will
     cause this pointer to be freed if it runs out of parents.
 
-  - you can talloc_free() the pointer itself. That will destroy the
-    most recently established parent to the pointer and leave the
-    pointer as a child of its current parent.
+  - you can talloc_free() the pointer itself if it has at maximum one
+    parent. This behaviour has been changed since the release of version
+    2.0. Further informations in the description of "talloc_free".
 
 For more control on which parent to remove, see talloc_unlink()
 
@@ -208,10 +204,10 @@ Note that if the parent has already been removed using talloc_free()
 then this function will fail and will return -1.  Likewise, if "ptr"
 is NULL, then the function will make no modifications and return -1.
 
-Usually you can just use talloc_free() instead of talloc_unlink(), but
-sometimes it is useful to have the additional control on which parent
-is removed.
-
+You can just use talloc_free() instead of talloc_unlink() if there
+is at maximum one parent. This behaviour has been changed since the
+release of version 2.0. Further informations in the description of
+"talloc_free".
 
 =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 void talloc_set_destructor(const void *ptr, int (*destructor)(void *));
@@ -483,7 +479,7 @@ been called.
 void talloc_report_full(const void *ptr, FILE *f);
 
 This provides a more detailed report than talloc_report(). It will
-recursively print the ensire tree of memory referenced by the
+recursively print the entire tree of memory referenced by the
 pointer. References in the tree are shown by giving the name of the
 pointer that is referenced.
 
@@ -642,7 +638,7 @@ char *talloc_asprintf_append(char *s, const char *fmt, ...);
 
 The talloc_asprintf_append() function appends the given formatted
 string to the given string.
-Use this varient when the string in the current talloc buffer may
+Use this variant when the string in the current talloc buffer may
 have been truncated in length.
 
 This functions sets the name of the new pointer to the new
@@ -656,7 +652,7 @@ char *talloc_asprintf_append_buffer(char *s, const char *fmt, ...);
 
 The talloc_asprintf_append() function appends the given formatted 
 string to the end of the currently allocated talloc buffer.
-Use this varient when the string in the current talloc buffer has
+Use this variant when the string in the current talloc buffer has
 not been changed.
 
 This functions sets the name of the new pointer to the new
@@ -730,7 +726,7 @@ this::
 =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 talloc_set_type(const void *ptr, type);
 
-This macro allows you to force the name of a pointer to be a
+This macro allows you to force the name of a pointer to be of a
 particular type. This can be used in conjunction with
 talloc_get_type() to do type checking on void* pointers.
 
@@ -741,7 +737,7 @@ It is equivalent to this::
 =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 talloc_get_size(const void *ctx);
 
-This function lets you know the amount of memory alloced so far by
+This function lets you know the amount of memory allocated so far by
 this context. It does NOT account for subcontext memory.
 This can be used to calculate the size of an array.
 
@@ -768,4 +764,4 @@ errors.
 =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 void talloc_set_log_stderr(void)
 
-This sets the talloc log function to write log messages to stderr
+This sets the talloc log function to write log messages to stderr.
diff --git a/lib/talloc/testsuite.c b/lib/talloc/testsuite.c
index 90417c6..003d74b 100644
--- a/lib/talloc/testsuite.c
+++ b/lib/talloc/testsuite.c
@@ -1317,6 +1317,49 @@ static bool test_rusty(void)
 	talloc_increase_ref_count(p1);
 	talloc_report_full(root, stdout);
 	talloc_free(root);
+	CHECK_BLOCKS("null_context", NULL, 2);
+	return true;
+}
+
+static bool test_free_children(void)
+{
+	void *root;
+	const char *p1, *p2, *name, *name2;
+
+	talloc_enable_null_tracking();
+	root = talloc_new(NULL);
+	p1 = talloc_strdup(root, "foo1");
+	p2 = talloc_strdup(p1, "foo2");
+
+	talloc_set_name(p1, "%s", "testname");
+	talloc_free_children(p1);
+	/* check its still a valid talloc ptr */
+	talloc_get_size(talloc_get_name(p1));
+	if (strcmp(talloc_get_name(p1), "testname") != 0) {
+		return false;
+	}
+
+	talloc_set_name(p1, "%s", "testname");
+	name = talloc_get_name(p1);
+	talloc_free_children(p1);
+	/* check its still a valid talloc ptr */
+	talloc_get_size(talloc_get_name(p1));
+	torture_assert("name", name == talloc_get_name(p1), "name ptr changed");
+	torture_assert("namecheck", strcmp(talloc_get_name(p1), "testname") == 0,
+		       "wrong name");
+	CHECK_BLOCKS("name1", p1, 2);
+
+	/* note that this does not free the old child name */
+	talloc_set_name_const(p1, "testname2");
+	name2 = talloc_get_name(p1);
+	/* but this does */
+	talloc_free_children(p1);
+	torture_assert("namecheck", strcmp(talloc_get_name(p1), "testname2") == 0,
+		       "wrong name");
+	CHECK_BLOCKS("name1", p1, 1);
+
+	talloc_report_full(root, stdout);
+	talloc_free(root);
 	return true;
 }
 
@@ -1379,6 +1422,8 @@ bool torture_local_talloc(struct torture_context *tctx)
 	ret &= test_free_ref_null_context();
 	test_reset();
 	ret &= test_rusty();
+	test_reset();
+	ret &= test_free_children();
 
 	if (ret) {
 		test_reset();


-- 
Samba Shared Repository


More information about the samba-cvs mailing list