talloc: make the no_owner feature simpler and working

metze

diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 140da5304767f7cc238b34ecc13153999761a2dd..cfa1cc99a7ae93f9616e214fb23617f94e43237c 100644 (file)
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -63,6 +63,7 @@
 #define TALLOC_FLAG_POOL 0x04          /* This is a talloc pool */
 #define TALLOC_FLAG_POOLMEM 0x08       /* This is allocated in a pool */
 #define TALLOC_MAGIC_REFERENCE ((const char *)1)
+#define TALLOC_MAGIC_NO_OWNER ((void *)1)
 
 /* by default we abort when given a bad pointer (such as when talloc_free() is called 
    on a pointer that came from malloc() */
@@ -103,8 +104,7 @@
 */
 static void *null_context;
 static void *autofree_context;
-static void *no_owner_context;
-void *talloc_no_owner_context(void);
+static void *no_owner_context = TALLOC_MAGIC_NO_OWNER;
 static inline int _talloc_free(void *ptr);
 
 struct talloc_reference_handle {
@@ -162,6 +162,11 @@ static void talloc_abort_double_free(void)
        talloc_abort("Bad talloc magic value - double free");
 }
 
+static void talloc_abort_no_owner_free(void)
+{
+       talloc_abort("Bad talloc parent - no_owner free");
+}
+
 static void talloc_abort_unknown_value(void)
 {
        talloc_abort("Bad talloc magic value - unknown value");
@@ -209,6 +214,23 @@ do { \
        if ((p) && ((p) != (list))) (p)->next = (p)->prev = NULL; \
 } while (0)
 
+static inline talloc_parent_is_no_owner(const void *ptr)
+{
+       struct talloc_chunk *tc;
+       struct talloc_chunk *no_owner_tc = TALLOC_MAGIC_NO_OWNER;
+
+       tc = talloc_chunk_from_ptr(ptr);
+
+       if (no_owner_context != TALLOC_MAGIC_NO_OWNER) {
+               no_owner_tc = talloc_chunk_from_ptr(no_owner_context);
+       }
+
+       if (tc->parent == no_owner_tc) {
+               return true;
+       }
+
+       return false;
+}
 
 /*
   return the parent chunk of a pointer
@@ -224,6 +246,10 @@ static inline struct talloc_chunk *talloc_parent_chunk(const void *ptr)
        tc = talloc_chunk_from_ptr(ptr);
        while (tc->prev) tc=tc->prev;
 
+       if (unlikely(tc->parent == TALLOC_MAGIC_NO_OWNER)) {
+               return NULL;
+       }
+
        return tc->parent;
 }
 
@@ -429,10 +455,17 @@ static int talloc_reference_destructor(struct talloc_reference_handle *handle)
 {
        struct talloc_chunk *ptr_tc = talloc_chunk_from_ptr(handle->ptr);
        _TLIST_REMOVE(ptr_tc->refs, handle);
-       /* If ptr_tc has no refs left and is owner by no_owner_context then we free it */
+       /*
+        * If ptr_tc has no refs left and is owner by
+        * no_owner_context then we free it
+        */
        if (ptr_tc->refs == NULL &&
-               no_owner_context && talloc_parent(handle->ptr) == no_owner_context) {
-                       _talloc_free(handle->ptr);
+           talloc_parent_is_no_owner(handle->ptr)) {
+               int ret = _talloc_free(handle->ptr);
+               if (ret != 0) {
+                       _TLIST_ADD(ptr_tc->refs, handle);
+                       return ret;
+               }
        }
        return 0;
 }
@@ -494,6 +527,41 @@ void *_talloc_reference(const void *context, const void *ptr)
        return handle->ptr;
 }
 
+static inline void _talloc_free_children(struct talloc_chunk *tc)
+{
+       void *ptr = TC_PTR_FROM_CHUNK(tc);
+
+       tc->flags |= TALLOC_FLAG_LOOP;
+
+       while (tc->child) {
+               void *child = TC_PTR_FROM_CHUNK(tc->child);
+               if (unlikely(_talloc_free(child) == -1)) {
+                       const void *new_parent = no_owner_context;
+                       struct talloc_chunk *p = talloc_parent_chunk(ptr);
+
+                       /*
+                        * we need to work out who will own an abandoned child
+                        * if it cannot be freed. In priority order, the first
+                        * choice is our parent, and the final choice is the
+                        * no owner context.
+                        */
+                       if (p) {
+                               new_parent = TC_PTR_FROM_CHUNK(p);
+                       }
+
+                       /*
+                        * if talloc_free fails because of refs and
+                        * not due to the destructor, it will already
+                        * have a new owner no_owner_context
+                        */
+                       if (!talloc_parent_is_no_owner(child)) {
+                               talloc_steal(new_parent, child);
+                       }
+               }
+       }
+
+       tc->flags &= ~TALLOC_FLAG_LOOP;
+}
 
 /* 
    internal talloc_free call
@@ -526,8 +594,11 @@ static inline int _talloc_free(void *ptr)
                        _talloc_free(tc->refs);
                        return _talloc_free(ptr);
                } else {
-                       /* we can't free if we have refs, so no_owner_context becomes the owner */
-                       _talloc_steal(talloc_no_owner_context(), ptr);
+                       /*
+                        * we can't free if we have refs,
+                        * so no_owner_context becomes the owner
+                        */
+                       _talloc_steal(no_owner_context, ptr);
                }
                return -1;
        }
@@ -560,17 +631,7 @@ static inline int _talloc_free(void *ptr)
                if (tc->next) tc->next->prev = tc->prev;
        }
 
-       tc->flags |= TALLOC_FLAG_LOOP;
-
-       while (tc->child) {
-               void *child = TC_PTR_FROM_CHUNK(tc->child);
-               const void *new_parent = talloc_no_owner_context();
-               /* if talloc_free fails because of refs and not due to the destructor,
-                  it will already have a new owner no_owner_context */
-               if (unlikely(_talloc_free(child) == -1)) {
-                       talloc_steal(new_parent, child);
-               }
-       }
+       _talloc_free_children(tc);
 
        tc->flags |= TALLOC_FLAG_FREE;
 
@@ -612,6 +673,15 @@ void *_talloc_steal(const void *new_ctx, const void *ptr)
                return NULL;
        }
 
+       if (unlikely(talloc_parent_is_no_owner(ptr) &&
+                    new_ctx != no_owner_context)) {
+               talloc_abort_no_owner_free();
+       }
+
+       if (unlikely(new_ctx == TALLOC_MAGIC_NO_OWNER)) {
+               new_ctx = NULL;
+       }
+
        if (unlikely(new_ctx == NULL)) {
                new_ctx = null_context;
        }
@@ -724,7 +794,7 @@ int talloc_unlink(const void *context, void *ptr)
                return _talloc_free(ptr);
        }
 
-       talloc_steal(talloc_no_owner_context(), ptr);
+       talloc_steal(no_owner_context, ptr);
        return 0;
 }
 
@@ -896,26 +966,7 @@ void talloc_free_children(void *ptr)
 
        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
-                  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 = talloc_no_owner_context();
-
-               if (unlikely(tc->child->refs)) {
-                       new_parent = talloc_no_owner_context();
-               }
-               if (unlikely(_talloc_free(child) == -1)) {
-                       if (new_parent == null_context) {
-                               struct talloc_chunk *p = talloc_parent_chunk(ptr);
-                               if (p) new_parent = TC_PTR_FROM_CHUNK(p);
-                       }
-                       talloc_steal(new_parent, child);
-               }
-       }
+       _talloc_free_children(tc);
 
        if ((tc->flags & TALLOC_FLAG_POOL)
            && (*talloc_pool_objectcount(tc) == 1)) {
@@ -971,8 +1022,13 @@ int talloc_free(void *ptr)
 
        tc = talloc_chunk_from_ptr(ptr);
 
-       if (no_owner_context && tc->parent == no_owner_context) {
-           talloc_abort_double_free();
+       if (unlikely(tc->flags & TALLOC_FLAG_LOOP)) {
+               /* we have a free loop - stop looping */
+               return 0;
+       }
+
+       if (unlikely(talloc_parent_is_no_owner(ptr))) {
+               talloc_abort_no_owner_free();
        }
 
        return _talloc_free(ptr);
@@ -1224,7 +1280,7 @@ static void talloc_report_depth_FILE_helper(const void *ptr, int depth, int max_
        FILE *f = (FILE *)_f;
 
        if (is_ref) {
-               if (talloc_parent(ptr) == no_owner_context && no_owner_context) {
+               if (talloc_parent_is_no_owner(ptr)) {
                        fprintf(f, "%*sreference to: %s [no owner]\n", depth*4, "", name);
                } else {
                        fprintf(f, "%*sreference to: %s\n", depth*4, "", name);
@@ -1317,6 +1373,8 @@ void talloc_enable_null_tracking(void)
 {
        if (null_context == NULL) {
                null_context = _talloc_named_const(NULL, 0, "null_context");
+               no_owner_context = _talloc_named_const(null_context, 0,
+                                                      "no_owner_context");
        }
 }
 
@@ -1327,6 +1385,7 @@ void talloc_disable_null_tracking(void)
 {
        _talloc_free(null_context);
        null_context = NULL;
+       no_owner_context = TALLOC_MAGIC_NO_OWNER;
 }
 
 /*
@@ -1724,29 +1783,6 @@ void *talloc_autofree_context(void)
        return autofree_context;
 }
 
-/* this is only used by the test suite so that failures from on test don't
-   confuse the results of the next test */
-void talloc_erase_no_owner_context(void)
-{
-       /* force free all children */
-       if (no_owner_context) {
-               struct talloc_chunk *tc;
-               tc = talloc_chunk_from_ptr(no_owner_context);
-               tc->child=NULL;
-               tc->refs=NULL;
-               _talloc_free(no_owner_context);
-               no_owner_context=NULL;
-       }
-}
-
-void *talloc_no_owner_context(void)
-{
-       if (no_owner_context == NULL) {
-               no_owner_context = _talloc_named_const(NULL, 0, "no_owner_context");
-       }
-       return no_owner_context;
-}
-
 size_t talloc_get_size(const void *context)
 {
        struct talloc_chunk *tc;