[PATCH] Add sticky references which are never promoted as parent or cause warning

Sam Liddicott sam at liddicott.com
Thu Jul 30 06:12:39 MDT 2009


Unless you anticipate your reference becoming parent when the original
reference is freed (and you need to be a wizard to properly anticipate
this) then you should be using talloc_reference_stick() and not
talloc_reference.

This patch stops non-sticky references from being promoted to parent
when the original parent is freed.

Non-sticky references do not cause warnings when talloc_steal or
talloc_free are used.
---
 lib/talloc/talloc.c |  138 ++++++++++++++++++++++++++++++++++++++++++---------
 lib/talloc/talloc.h |    1 +
 2 files changed, 116 insertions(+), 23 deletions(-)

diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index a5fe14d..62db4f6 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -7,6 +7,7 @@
 
    Copyright (C) Andrew Tridgell 2004
    Copyright (C) Stefan Metzmacher 2006
+   Copyright (C) Sam Liddicott 2009
    
      ** NOTE! The following LGPL license applies to the talloc
      ** library. This does NOT imply that all of Samba is released
@@ -65,6 +66,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 talloc_no_owner_context()
 
 /* by default we abort when given a bad pointer (such as when talloc_free() is called 
    on a pointer that came from malloc() */
@@ -105,10 +107,14 @@
 */
 static void *null_context;
 static void *autofree_context;
+static void *no_owner_context;
+void *talloc_no_owner_context(void);
+static inline int _talloc_free_internal(void *ptr);
 
 struct talloc_reference_handle {
 	struct talloc_reference_handle *next, *prev;
 	void *ptr;
+	bool non_stick;
 	const char *location;
 };
 
@@ -282,6 +288,24 @@ const char *talloc_parent_name(const void *ptr)
 }
 
 /*
+  non_stick_refs - returns the first of tc->refs that has non_stick == true
+*/
+static struct talloc_reference_handle *non_stick_refs(const struct talloc_chunk *tc)
+{
+	struct talloc_reference_handle *h;
+
+	if (unlikely(tc == NULL)) {
+		return NULL;
+	}
+
+	for (h=tc->refs;h;h=h->next) {
+		if (h->non_stick) return h;
+	}
+	return NULL;
+}
+
+
+/*
   A pool carries an in-pool object count count in the first 16 bytes.
   bytes. This is done to support talloc_steal() to a parent outside of the
   pool. The count includes the pool itself, so a talloc_free() on a pool will
@@ -449,11 +473,11 @@ void _talloc_set_destructor(const void *ptr, int (*destructor)(void *))
 }
 
 /*
-  increase the reference count on a piece of memory. 
+  increase the reference count on a piece of memory, using sticky references.
 */
 int talloc_increase_ref_count(const void *ptr)
 {
-	if (unlikely(!talloc_reference(null_context, ptr))) {
+	if (unlikely(!talloc_reference_sticky(null_context, ptr))) {
 		return -1;
 	}
 	return 0;
@@ -468,6 +492,12 @@ 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 owned by TALLOC_MAGIC_NO_OWNER then we free it */
+	if (ptr_tc->refs == NULL &&
+	    ptr_tc->parent == TALLOC_MAGIC_NO_OWNER) {
+		_talloc_free_internal(handle->ptr);
+	}
 	return 0;
 }
 
@@ -507,7 +537,8 @@ static inline void *_talloc_named_const(const void *context, size_t size, const
   same underlying data, and you want to be able to free the two instances separately,
   and in either order
 */
-void *_talloc_reference_loc(const void *context, const void *ptr, const char *location)
+static void *_talloc_reference_loc_internal(const void *context, const void *ptr,
+				     const char *location, bool non_stick)
 {
 	struct talloc_chunk *tc;
 	struct talloc_reference_handle *handle;
@@ -524,11 +555,22 @@ void *_talloc_reference_loc(const void *context, const void *ptr, const char *lo
 	   own destructor on the context if they want to */
 	talloc_set_destructor(handle, talloc_reference_destructor);
 	handle->ptr = discard_const_p(void, ptr);
+	handle->non_stick = non_stick;
 	handle->location = location;
 	_TLIST_ADD(tc->refs, handle);
 	return handle->ptr;
 }
 
+void *_talloc_reference_loc(const void *context, const void *ptr, const char *location)
+{
+	return _talloc_reference_loc_internal(context, ptr, location, true);
+}
+
+void *_talloc_reference_loc_sticky(const void *context, const void *ptr, const char *location)
+{
+	return _talloc_reference_loc_internal(context, ptr, location, false);
+}
+
 static void *_talloc_steal_internal(const void *new_ctx, const void *ptr);
 
 /* 
@@ -537,14 +579,17 @@ static void *_talloc_steal_internal(const void *new_ctx, const void *ptr);
 static inline int _talloc_free_internal(void *ptr)
 {
 	struct talloc_chunk *tc;
+	struct talloc_reference_handle *h;
 
 	if (unlikely(ptr == NULL)) {
 		return -1;
 	}
 
 	tc = talloc_chunk_from_ptr(ptr);
+	h = non_stick_refs(tc);
 
-	if (unlikely(tc->refs)) {
+	/* Free non_stick references rather than the parent */
+	if (unlikely(h)) {
 		int is_child;
 		/* check this is a reference from a child or grantchild
 		 * back to it's parent or grantparent
@@ -553,11 +598,24 @@ static inline int _talloc_free_internal(void *ptr)
 		 * call another instance of talloc_free() on the current
 		 * pointer.
 		 */
-		is_child = talloc_is_parent(tc->refs, ptr);
-		_talloc_free_internal(tc->refs);
+		is_child = talloc_is_parent(h, ptr);
+		_talloc_free_internal(h);
 		if (is_child) {
 			return _talloc_free_internal(ptr);
 		}
+		/* Free-ing a non_stick pointer is normally sufficient,
+	           but if there are no refs remaining and the parent is not
+		   TALLOC_MAGIC_NO_OWNER then we don't return yet, but
+		   carry on to finish the free. */
+		if (tc->refs || tc->parent != TALLOC_MAGIC_NO_OWNER) {
+			return -1;
+		}
+	}
+
+	/* Even though the code above removes non_stick references, we may
+	   still have sticky references at this point */
+	if (tc->refs) {
+		_talloc_steal_internal(TALLOC_MAGIC_NO_OWNER, ptr);
 		return -1;
 	}
 
@@ -599,8 +657,18 @@ static inline int _talloc_free_internal(void *ptr)
 		   final choice is the null context. */
 		void *child = TC_PTR_FROM_CHUNK(tc->child);
 		const void *new_parent = null_context;
-		if (unlikely(tc->child->refs)) {
-			struct talloc_chunk *p = talloc_parent_chunk(tc->child->refs);
+		struct talloc_reference_handle *ch;
+
+		/* If child has refs, then make the new owner be no_owner_context
+		   in order to help it free when it's refs free */
+		if (tc->refs) {
+			new_parent = TALLOC_MAGIC_NO_OWNER;
+		}
+
+		/* Only promote non-stick refs to parent */
+		ch = non_stick_refs(tc->child);
+		if (unlikely(ch)) {
+			struct talloc_chunk *p = talloc_parent_chunk(ch);
 			if (p) new_parent = TC_PTR_FROM_CHUNK(p);
 		}
 		if (unlikely(_talloc_free_internal(child) == -1)) {
@@ -705,24 +773,27 @@ static void *_talloc_steal_internal(const void *new_ctx, const void *ptr)
 void *_talloc_steal_loc(const void *new_ctx, const void *ptr, const char *location)
 {
 	struct talloc_chunk *tc;
+	struct talloc_reference_handle *h;
 
 	if (unlikely(ptr == NULL)) {
 		return NULL;
 	}
 	
 	tc = talloc_chunk_from_ptr(ptr);
+	h = non_stick_refs(tc);
 	
-	if (unlikely(tc->refs != NULL) && talloc_parent(ptr) != new_ctx) {
-		struct talloc_reference_handle *h;
+	if (unlikely(h != NULL) && talloc_parent(ptr) != new_ctx) {
 		char *msg;
 
 		msg = talloc_asprintf(NULL,
-			"WARNING: talloc_steal with references at %s\n",
+			"WARNING: talloc_steal with non-stick references at %s\n",
 			location);
 		talloc_log(msg);
 		talloc_free(msg);
 
 		for (h=tc->refs; h; h=h->next) {
+			if (h->non_stick) continue;
+
 			msg = talloc_asprintf(NULL,
 				"\treference at %s\n",
 				h->location);
@@ -830,21 +901,21 @@ int talloc_unlink(const void *context, void *ptr)
 	
 	tc_p = talloc_chunk_from_ptr(ptr);
 
+	/* If we have no references, then free */
 	if (tc_p->refs == NULL) {
 		return _talloc_free_internal(ptr);
 	}
 
-	new_p = talloc_parent_chunk(tc_p->refs);
+	/* Only promote non-stick references to parent */
+	new_p = talloc_parent_chunk(non_stick_refs(tc_p));
 	if (new_p) {
 		new_parent = TC_PTR_FROM_CHUNK(new_p);
+		if (talloc_unreference(new_parent, ptr) != 0) {
+			return -1;
+		}
 	} else {
-		new_parent = NULL;
+		new_parent = TALLOC_MAGIC_NO_OWNER;
 	}
-
-	if (talloc_unreference(new_parent, ptr) != 0) {
-		return -1;
-	}
-
 	_talloc_steal_internal(new_parent, ptr);
 
 	return 0;
@@ -1026,8 +1097,17 @@ 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;
-		if (unlikely(tc->child->refs)) {
-			struct talloc_chunk *p = talloc_parent_chunk(tc->child->refs);
+		struct talloc_reference_handle *ch;
+
+		/* If child has refs, then make the new owner be no_owner_context
+		   in order to help it free when it's refs free */
+		if (tc->refs) {
+			new_parent = TALLOC_MAGIC_NO_OWNER;
+		}
+		/* Only promote non-stick refs to parent */
+		ch = non_stick_refs(tc->child);
+		if (unlikely(ch)) {
+			struct talloc_chunk *p = talloc_parent_chunk(ch);
 			if (p) new_parent = TC_PTR_FROM_CHUNK(p);
 		}
 		if (unlikely(talloc_free(child) == -1)) {
@@ -1086,24 +1166,28 @@ void *talloc_named_const(const void *context, size_t size, const char *name)
 int _talloc_free(void *ptr, const char *location)
 {
 	struct talloc_chunk *tc;
+	struct talloc_reference_handle *h;
 
 	if (unlikely(ptr == NULL)) {
 		return -1;
 	}
 	
 	tc = talloc_chunk_from_ptr(ptr);
+	h = non_stick_refs(tc);
 	
-	if (unlikely(tc->refs != NULL)) {
+	if (unlikely(h != NULL)) {
 		struct talloc_reference_handle *h;
 		char *msg;
 
 		msg = talloc_asprintf(NULL,
-			"ERROR: talloc_free with references at %s\n",
+			"ERROR: talloc_free with non-stick references at %s\n",
 			location);
 		talloc_log(msg);
 		talloc_free(msg);
 
-		for (h=tc->refs; h; h=h->next) {
+		for (; h; h=h->next) {
+			if (h->non_stick) continue;
+
 			msg = talloc_asprintf(NULL,
 				"\treference at %s\n",
 				h->location);
@@ -1858,6 +1942,14 @@ void *talloc_autofree_context(void)
 	return autofree_context;
 }
 
+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;
diff --git a/lib/talloc/talloc.h b/lib/talloc/talloc.h
index 234c0b4..e59aedf 100644
--- a/lib/talloc/talloc.h
+++ b/lib/talloc/talloc.h
@@ -78,6 +78,7 @@ typedef void TALLOC_CTX;
 #endif
 
 #define talloc_reference(ctx, ptr) (_TALLOC_TYPEOF(ptr))_talloc_reference_loc((ctx),(ptr), __location__)
+#define talloc_reference_sticky(ctx, ptr) (_TALLOC_TYPEOF(ptr))_talloc_reference_loc((ctx),(ptr), __location__)
 #define talloc_move(ctx, ptr) (_TALLOC_TYPEOF(*(ptr)))_talloc_move((ctx),(void *)(ptr))
 
 /* useful macros for creating type checked pointers */
-- 
1.6.0.3


--------------030904060801050202020707--


More information about the samba-technical mailing list