The Samba-Bugzilla – Attachment 6448 Details for
Bug 8140
talloc: valgrind false positives and other backports
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Backport Patches for v3-6-test
tmp.diff (text/plain), 21.52 KB, created by
Stefan Metzmacher
on 2011-05-17 09:45:42 UTC
(
hide
)
Description:
Backport Patches for v3-6-test
Filename:
MIME Type:
Creator:
Stefan Metzmacher
Created:
2011-05-17 09:45:42 UTC
Size:
21.52 KB
patch
obsolete
>From e703c7b91dc3ef08f875fca14eb037174fc10c30 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 16 May 2011 19:06:07 +0200 >Subject: [PATCH 1/9] talloc: split the handling of FLAG_POOL/FLAG_POOLMEM in _talloc_free_internal > >The optimization of the object_count == 1 case should only happen >for when we're not destroying the pool itself. And it should only >happen if the pool itself is still valid. > >If the pool isn't valid (it has TALLOC_FLAG_FREE), >object_count == 1 does not mean that the pool is the last object, >which can happen if you use talloc_steal/move() on memory >from the pool and then free the pool itself. > >Thanks to Volker for noticing this! > >metze >(cherry picked from commit 2d514be1ed3b8245157a0a51186ec7f9db828202) >--- > lib/talloc/talloc.c | 98 ++++++++++++++++++++++++++++++++++---------------- > 1 files changed, 66 insertions(+), 32 deletions(-) > >diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c >index 2a956dc..0efeb7a 100644 >--- a/lib/talloc/talloc.c >+++ b/lib/talloc/talloc.c >@@ -709,6 +709,65 @@ _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; >+ } >+} >+ > /* > internal talloc_free call > */ >@@ -823,21 +882,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!"); >@@ -846,26 +894,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); >-- >1.7.0.4 > > >From 308917092746ea81b6ef31ef674234184264ac3a Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 16 May 2011 20:23:13 +0200 >Subject: [PATCH 2/9] talloc: make use of _talloc_free_poolmem() in _talloc_realloc() > >This should follow the same logic... > >metze >(cherry picked from commit 14b662ee4f278764b9dfd620851e908d29f29fc4) >--- > lib/talloc/talloc.c | 16 +--------------- > 1 files changed, 1 insertions(+), 15 deletions(-) > >diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c >index 0efeb7a..fcd86d7 100644 >--- a/lib/talloc/talloc.c >+++ b/lib/talloc/talloc.c >@@ -1550,7 +1550,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); >@@ -1559,21 +1558,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 { >-- >1.7.0.4 > > >From f768df01bcdf80c552a5de0b3086778c9ca98af9 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 16 May 2011 20:15:59 +0200 >Subject: [PATCH 3/9] talloc: make really sure only optimize realloc if there's only one pool chunk > >*talloc_pool_objectcount(pool_tc) == 2 doesn't mean the one of the objects >is the pool itself! So we better check for == 1 and calculate the chunk count. > >metze >(cherry picked from commit 7102105c8954627dc30a851327cf2642ac0783d5) >--- > lib/talloc/talloc.c | 7 ++++++- > 1 files changed, 6 insertions(+), 1 deletions(-) > >diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c >index fcd86d7..f3ed9c8 100644 >--- a/lib/talloc/talloc.c >+++ b/lib/talloc/talloc.c >@@ -1479,8 +1479,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 (*talloc_pool_objectcount(pool_tc) == 2) { >+ if (!(pool_tc->flags & TALLOC_FLAG_FREE)) { >+ chunk_count -= 1; >+ } >+ >+ if (chunk_count == 1) { > /* > * optimize for the case where 'tc' is the only > * chunk in the pool. >-- >1.7.0.4 > > >From ba46ab77bf521e3e1c73fc65b97c64d214f329c5 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Tue, 17 May 2011 08:19:04 +0200 >Subject: [PATCH 4/9] talloc: setup the new 'tc' before TC_UNDEFINE_GROW_CHUNK() _talloc_realloc() > >metze >(cherry picked from commit c281f2fc1a359d0d3b91b94438f11bb7c88170b5) >--- > lib/talloc/talloc.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > >diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c >index f3ed9c8..9553405 100644 >--- a/lib/talloc/talloc.c >+++ b/lib/talloc/talloc.c >@@ -1512,6 +1512,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); > > /* >-- >1.7.0.4 > > >From 58daf6d04013af68543c926a373f6108b22e7bee Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Tue, 17 May 2011 08:20:13 +0200 >Subject: [PATCH 5/9] talloc: add memset() calls to test_pool() > >This way we the pool based valgrind code. > >metze >(cherry picked from commit 16cc52cf70a9918843f9761baf483338c80bf1d0) >--- > lib/talloc/testsuite.c | 17 +++++++++++++++++ > 1 files changed, 17 insertions(+), 0 deletions(-) > >diff --git a/lib/talloc/testsuite.c b/lib/talloc/testsuite.c >index ba583ab..1408545 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 */ > >-- >1.7.0.4 > > >From eed287d5c88d37d4cdd65da6ec7d5db7c03285cf Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 16 May 2011 19:25:47 +0200 >Subject: [PATCH 6/9] talloc: test talloc_steal out of a talloc_pool > >metze > >Autobuild-User: Stefan Metzmacher <metze@samba.org> >Autobuild-Date: Tue May 17 09:43:01 CEST 2011 on sn-devel-104 >(cherry picked from commit 37b2130ed9612a7334888ecd2fee26b0b45ac271) >--- > lib/talloc/testsuite.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 69 insertions(+), 0 deletions(-) > >diff --git a/lib/talloc/testsuite.c b/lib/talloc/testsuite.c >index 1408545..90417c6 100644 >--- a/lib/talloc/testsuite.c >+++ b/lib/talloc/testsuite.c >@@ -1208,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) > { >@@ -1307,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(); >-- >1.7.0.4 > > >From d90c98f09dc92a9e2703efad7eed07c5120101b6 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Fri, 8 Apr 2011 12:27:05 +0200 >Subject: [PATCH 7/9] talloc: use _talloc_free_internal() in talloc_free_children() > >metze >(cherry picked from commit f3b855d2ff9576715afe50d75678829c6bc0842d) >--- > lib/talloc/talloc.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > >diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c >index 9553405..3130858 100644 >--- a/lib/talloc/talloc.c >+++ b/lib/talloc/talloc.c >@@ -1291,7 +1291,7 @@ _PUBLIC_ void talloc_free_children(void *ptr) > 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)) { >+ if (unlikely(_talloc_free_internal(child, __location__) == -1)) { > if (new_parent == null_context) { > struct talloc_chunk *p = talloc_parent_chunk(ptr); > if (p) new_parent = TC_PTR_FROM_CHUNK(p); >-- >1.7.0.4 > > >From 3677615e53ab3422882221a6d72b164679a16c98 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Fri, 8 Apr 2011 12:30:46 +0200 >Subject: [PATCH 8/9] talloc: fixed a use after free error in talloc_free_children() > >This is similar to commit 6f51a1f45bf4de062cce7a562477e8140630a53d. > >metze >(cherry picked from commit 38633c9f0b7f86673f08903999583ad5b62c3548) >--- > lib/talloc/talloc.c | 17 ++++++++++++++++- > 1 files changed, 16 insertions(+), 1 deletions(-) > >diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c >index 3130858..8333f41 100644 >--- a/lib/talloc/talloc.c >+++ b/lib/talloc/talloc.c >@@ -1287,13 +1287,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); > } >+ /* 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); >-- >1.7.0.4 > > >From 6a349489bb8f604604f3e8d5e84ef50066aea625 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Tue, 17 May 2011 08:50:45 +0200 >Subject: [PATCH 9/9] talloc: splitout _talloc_free_children_internal() > >metze > >Autobuild-User: Stefan Metzmacher <metze@samba.org> >Autobuild-Date: Tue May 17 10:49:13 CEST 2011 on sn-devel-104 >(cherry picked from commit df2cb2f672569e5d113fe2e77fdc1ee16c8b646d) >--- > lib/talloc/talloc.c | 77 ++++++++++++++++++--------------------------------- > 1 files changed, 27 insertions(+), 50 deletions(-) > >diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c >index 8333f41..4700aa9 100644 >--- a/lib/talloc/talloc.c >+++ b/lib/talloc/talloc.c >@@ -768,6 +768,10 @@ static inline void _talloc_free_poolmem(struct talloc_chunk *tc, > } > } > >+static inline void _talloc_free_children_internal(struct talloc_chunk *tc, >+ void *ptr, >+ const char *location); >+ > /* > internal talloc_free call > */ >@@ -838,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; > >@@ -1264,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 >@@ -1306,7 +1265,7 @@ _PUBLIC_ void talloc_free_children(void *ptr) > 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 (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); >@@ -1316,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 > */ >-- >1.7.0.4 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=6448">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
vl
:
review+
Actions:
View
Attachments on
bug 8140
: 6448