[PATCH] Attempt to bring in 5fe1d8dc1275e43d96da1297f5fb0d0088a1c3ab: changes to remove the ambiguity in talloc_free() and talloc_steal()
Sam Liddicott
sam at liddicott.com
Wed Jul 29 02:26:13 MDT 2009
The main change is introduction of slippy_ref(tc) which finds
the first non-sticky references.
These are old-style references where it is a crime to call
talloc_free if you have more than one.
The use of stderr in the logging is ugly (and potentially dangerous),
and will be removed in a future patch. We'll need to add a debug
registration function to talloc.
Conflicts:
lib/talloc/talloc.c
lib/talloc/talloc.h
---
lib/talloc/talloc.c | 161 +++++++++++++++++++++++++++++++++++++++++----------
lib/talloc/talloc.h | 18 ++++--
2 files changed, 142 insertions(+), 37 deletions(-)
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 9f21ca4..1b3f353 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -106,12 +106,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(void *ptr);
+int _talloc_free(void *ptr, const char *location);
+void *_talloc_reference_sticky(const void *context, const void *ptr, const char *location);
struct talloc_reference_handle {
struct talloc_reference_handle *next, *prev;
void *ptr;
bool sticky;
+ const char *location;
};
typedef int (*talloc_destructor_t)(void *);
@@ -245,6 +247,24 @@ const char *talloc_parent_name(const void *ptr)
}
/*
+ slippy_ref - returns the first of tc->refs that has slippy == true
+*/
+static struct talloc_reference_handle *slippy_ref(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->sticky) 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
@@ -445,7 +465,7 @@ static int talloc_reference_destructor(struct talloc_reference_handle *handle)
/* 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_internal_free(handle->ptr);
}
return 0;
}
@@ -486,7 +506,7 @@ 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(const void *context, const void *ptr, bool sticky)
+void *__talloc_reference(const void *context, const void *ptr, bool sticky, const char *location)
{
struct talloc_chunk *tc;
struct talloc_reference_handle *handle;
@@ -503,25 +523,26 @@ void *__talloc_reference(const void *context, const void *ptr, bool sticky)
own destructor on the context if they want to */
talloc_set_destructor(handle, talloc_reference_destructor);
handle->ptr = discard_const_p(void, ptr);
+ handle->location = location;
_TLIST_ADD(tc->refs, handle);
handle->sticky = sticky;
return handle->ptr;
}
-void *_talloc_reference(const void *context, const void *ptr)
+void *_talloc_reference(const void *context, const void *ptr, const char *location)
{
- return __talloc_reference(context, ptr, false);
+ return __talloc_reference(context, ptr, false, location);
}
-void *_talloc_reference_sticky(const void *context, const void *ptr)
+void *_talloc_reference_sticky(const void *context, const void *ptr, const char *location)
{
- return __talloc_reference(context, ptr, false);
+ return __talloc_reference(context, ptr, false, location);
}
/*
internal talloc_free call
*/
-static inline int _talloc_free(void *ptr)
+static inline int _talloc_free_internal(void *ptr)
{
struct talloc_chunk *tc;
@@ -546,11 +567,11 @@ static inline int _talloc_free(void *ptr)
*/
is_child = talloc_is_parent(tc->refs, ptr);
if (is_child) {
- _talloc_free(tc->refs);
- return _talloc_free(ptr);
+ _talloc_free_internal(tc->refs);
+ return _talloc_free_internal(ptr);
} else {
/* we can't free if we have refs, so no_owner_context becomes the owner */
- _talloc_steal(talloc_no_owner_context(), ptr);
+ _talloc_steal_internal(talloc_no_owner_context(), ptr);
}
return -1;
}
@@ -587,11 +608,21 @@ static inline int _talloc_free(void *ptr)
while (tc->child) {
void *child = TC_PTR_FROM_CHUNK(tc->child);
- const void *new_parent = talloc_no_owner_context();
+ const void *new_parent = new_parent = talloc_no_owner_context();;
+ const void* ref = slippy_ref(tc->child);
+
+ if (unlikely(ref)) {
+ struct talloc_chunk *p = talloc_parent_chunk(ref);
+ 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 (unlikely(_talloc_free(child) == -1)) {
- talloc_steal(new_parent, child);
+ if (unlikely(_talloc_free_internal(child) == -1)) {
+ if (new_parent == null_context || new_parent == talloc_no_owner_context()) {
+ struct talloc_chunk *p = talloc_parent_chunk(ptr);
+ if (p) new_parent = TC_PTR_FROM_CHUNK(p);
+ }
+ talloc_steal_internal(new_parent, child);
}
}
@@ -627,7 +658,7 @@ static inline int _talloc_free(void *ptr)
ptr on success, or NULL if it could not be transferred.
passing NULL as ptr will always return NULL with no side effects.
*/
-void *_talloc_steal(const void *new_ctx, const void *ptr)
+void *_talloc_steal_internal(const void *new_ctx, const void *ptr)
{
struct talloc_chunk *tc, *new_tc;
@@ -680,6 +711,66 @@ void *_talloc_steal(const void *new_ctx, const void *ptr)
}
+/*
+ move a lump of memory from one talloc context to another return the
+ ptr on success, or NULL if it could not be transferred.
+ passing NULL as ptr will always return NULL with no side effects.
+*/
+void *_talloc_steal(const void *new_ctx, const void *ptr, const char *location)
+{
+ struct talloc_chunk *tc;
+
+ if (unlikely(ptr == NULL)) {
+ return NULL;
+ }
+
+ tc = talloc_chunk_from_ptr(ptr);
+
+ if (unlikely(tc->refs != NULL) && talloc_parent(ptr) != new_ctx) {
+ struct talloc_reference_handle *h;
+ fprintf(stderr, "ERROR: talloc_steal with references at %s\n", location);
+ for (h=tc->refs; h; h=h->next) {
+ fprintf(stderr, "\treference at %s\n", h->location);
+ }
+ return NULL;
+ }
+
+ return _talloc_steal_internal(new_ctx, ptr);
+}
+
+/*
+ this is like a talloc_steal(), but you must supply the old
+ parent. This resolves the ambiguity in a talloc_steal() which is
+ called on a context that has more than one parent (via references)
+
+ The old parent can be either a reference or a parent
+*/
+void *talloc_reparent(const void *old_parent, const void *new_parent, const void *ptr)
+{
+ struct talloc_chunk *tc;
+ struct talloc_reference_handle *h;
+
+ if (unlikely(ptr == NULL)) {
+ return NULL;
+ }
+
+ if (old_parent == talloc_parent(ptr)) {
+ return _talloc_steal_internal(new_parent, ptr);
+ }
+
+ tc = talloc_chunk_from_ptr(ptr);
+ for (h=tc->refs;h;h=h->next) {
+ if (talloc_parent(h) == old_parent) {
+ if (_talloc_steal_internal(new_parent, h) != h) {
+ return NULL;
+ }
+ return ptr;
+ }
+ }
+
+ /* it wasn't a parent */
+ return NULL;
+}
/*
remove a secondary reference to a pointer. This undo's what
@@ -707,7 +798,7 @@ static inline int talloc_unreference(const void *context, const void *ptr)
return -1;
}
- return _talloc_free(h);
+ return _talloc_free_internal(h);
}
/*
@@ -744,10 +835,11 @@ int talloc_unlink(const void *context, void *ptr)
tc_p = talloc_chunk_from_ptr(ptr);
if (tc_p->refs == NULL) {
- return _talloc_free(ptr);
+ return _talloc_free_internal(ptr);
}
- talloc_steal(talloc_no_owner_context(), ptr);
+ talloc_steal_internal(talloc_no_owner_context(), ptr);
+
return 0;
}
@@ -799,7 +891,7 @@ void *talloc_named(const void *context, size_t size, const char *fmt, ...)
va_end(ap);
if (unlikely(name == NULL)) {
- _talloc_free(ptr);
+ _talloc_free_internal(ptr);
return NULL;
}
@@ -897,7 +989,7 @@ void *talloc_init(const char *fmt, ...)
va_end(ap);
if (unlikely(name == NULL)) {
- _talloc_free(ptr);
+ _talloc_free_internal(ptr);
return NULL;
}
@@ -931,12 +1023,12 @@ void talloc_free_children(void *ptr)
if (unlikely(tc->child->refs)) {
new_parent = talloc_no_owner_context();
}
- if (unlikely(_talloc_free(child) == -1)) {
+ 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_steal_internal(new_parent, child);
}
}
@@ -984,7 +1076,7 @@ void *talloc_named_const(const void *context, size_t size, const char *name)
will not be freed if the ref_count is > 1 or the destructor (if
any) returns non-zero
*/
-int talloc_free(void *ptr)
+int _talloc_free(void *ptr, const char *location)
{
struct talloc_chunk *tc;
@@ -994,15 +1086,24 @@ int talloc_free(void *ptr)
tc = talloc_chunk_from_ptr(ptr);
+ /* if we have any non-sticky tc->refs, then error */
+ if (unlikely(slippy_ref(tc) != NULL)) {
+ struct talloc_reference_handle *h;
+ fprintf(stderr, "ERROR: talloc_free with references at %s\n", location);
+ for (h=tc->refs; h; h=h->next) {
+ fprintf(stderr, "\treference at %s\n", h->location);
+ }
+ return -1;
+ }
+
if (no_owner_context && tc->parent == no_owner_context) {
talloc_abort_double_free();
}
- return _talloc_free(ptr);
+ return _talloc_free_internal(ptr);
}
-
/*
A talloc version of realloc. The context argument is only used if
ptr is NULL
@@ -1015,7 +1116,7 @@ void *_talloc_realloc(const void *context, void *ptr, size_t size, const char *n
/* size zero is equivalent to free() */
if (unlikely(size == 0)) {
- _talloc_free(ptr);
+ talloc_free(ptr);
return NULL;
}
@@ -1112,7 +1213,7 @@ void *_talloc_realloc(const void *context, void *ptr, size_t size, const char *n
void *_talloc_move(const void *new_ctx, const void *_pptr)
{
const void **pptr = discard_const_p(const void *,_pptr);
- void *ret = _talloc_steal(new_ctx, *pptr);
+ void *ret = talloc_steal(new_ctx, *pptr);
(*pptr) = NULL;
return ret;
}
@@ -1337,7 +1438,7 @@ void talloc_enable_null_tracking(void)
*/
void talloc_disable_null_tracking(void)
{
- _talloc_free(null_context);
+ talloc_free(null_context);
null_context = NULL;
}
@@ -1719,7 +1820,7 @@ static int talloc_autofree_destructor(void *ptr)
static void talloc_autofree(void)
{
- _talloc_free(autofree_context);
+ talloc_free(autofree_context);
}
/*
@@ -1746,7 +1847,7 @@ void talloc_erase_no_owner_context(void)
tc = talloc_chunk_from_ptr(no_owner_context);
tc->child=NULL;
tc->refs=NULL;
- _talloc_free(no_owner_context);
+ _talloc_free_internal(no_owner_context);
no_owner_context=NULL;
}
}
diff --git a/lib/talloc/talloc.h b/lib/talloc/talloc.h
index 8150cea..3ce1255 100644
--- a/lib/talloc/talloc.h
+++ b/lib/talloc/talloc.h
@@ -69,16 +69,16 @@ typedef void TALLOC_CTX;
} while(0)
/* this extremely strange macro is to avoid some braindamaged warning
stupidity in gcc 4.1.x */
-#define talloc_steal(ctx, ptr) ({ _TALLOC_TYPEOF(ptr) __talloc_steal_ret = (_TALLOC_TYPEOF(ptr))_talloc_steal((ctx),(ptr)); __talloc_steal_ret; })
+#define talloc_steal(ctx, ptr) ({ _TALLOC_TYPEOF(ptr) __talloc_steal_ret = (_TALLOC_TYPEOF(ptr))_talloc_steal((ctx),(ptr), __location__); __talloc_steal_ret; })
#else
#define talloc_set_destructor(ptr, function) \
_talloc_set_destructor((ptr), (int (*)(void *))(function))
#define _TALLOC_TYPEOF(ptr) void *
-#define talloc_steal(ctx, ptr) (_TALLOC_TYPEOF(ptr))_talloc_steal((ctx),(ptr))
+#define talloc_steal(ctx, ptr) (_TALLOC_TYPEOF(ptr))_talloc_steal((ctx),(ptr), __location__)
#endif
-#define talloc_reference(ctx, ptr) (_TALLOC_TYPEOF(ptr))_talloc_reference((ctx),(ptr))
-#define talloc_reference_sticky(ctx, ptr) (_TALLOC_TYPEOF(ptr))_talloc_reference_sticky((ctx),(ptr))
+#define talloc_reference(ctx, ptr) (_TALLOC_TYPEOF(ptr))_talloc_reference((ctx),(ptr), __location__)
+#define talloc_reference_sticky(ctx, ptr) (_TALLOC_TYPEOF(ptr))_talloc_reference_sticky((ctx),(ptr), __location__)
#define talloc_move(ctx, ptr) (_TALLOC_TYPEOF(*(ptr)))_talloc_move((ctx),(void *)(ptr))
/* useful macros for creating type checked pointers */
@@ -107,6 +107,8 @@ typedef void TALLOC_CTX;
#define talloc_get_type_abort(ptr, type) (type *)_talloc_get_type_abort(ptr, #type, __location__)
#define talloc_find_parent_bytype(ptr, type) (type *)talloc_find_parent_byname(ptr, #type)
+#define talloc_free(ctx) _talloc_free(ctx, __location__)
+
#if TALLOC_DEPRECATED
#define talloc_zero_p(ctx, type) talloc_zero(ctx, type)
@@ -126,7 +128,7 @@ void _talloc_set_destructor(const void *ptr, int (*_destructor)(void *));
int talloc_increase_ref_count(const void *ptr);
int talloc_increase_ref_count_sticky(const void *ptr);
size_t talloc_reference_count(const void *ptr);
-void *_talloc_reference(const void *context, const void *ptr);
+void *_talloc_reference(const void *context, const void *ptr, const char *location);
int talloc_unlink(const void *context, void *ptr);
const char *talloc_set_name(const void *ptr, const char *fmt, ...) PRINTF_ATTRIBUTE(2,3);
void talloc_set_name_const(const void *ptr, const char *name);
@@ -139,10 +141,12 @@ void *_talloc_get_type_abort(const void *ptr, const char *name, const char *loca
void *talloc_parent(const void *ptr);
const char *talloc_parent_name(const void *ptr);
void *talloc_init(const char *fmt, ...) PRINTF_ATTRIBUTE(1,2);
-int talloc_free(void *ptr);
+int _talloc_free(void *ptr, const char *location);
void talloc_free_children(void *ptr);
void *_talloc_realloc(const void *context, void *ptr, size_t size, const char *name);
-void *_talloc_steal(const void *new_ctx, const void *ptr);
+void *_talloc_steal(const void *new_ctx, const void *ptr, const char *location);
+void *_talloc_steal_internal(const void *new_ctx, const void *ptr);
+void *talloc_reparent(const void *old_parent, const void *new_parent, const void *ptr);
void *_talloc_move(const void *new_ctx, const void *pptr);
size_t talloc_total_size(const void *ptr);
size_t talloc_total_blocks(const void *ptr);
--
1.6.0.3
--------------030508000909030907000202--
More information about the samba-technical
mailing list