Hi Juergen,
On 01/11/2022 15:28, Juergen Gross wrote:
Today talloc_free() is not guaranteed to preserve errno, especially in
case a custom destructor is being used.
Change that by renaming talloc_free() to _talloc_free() in talloc.c and
adding a wrapper to talloc.c.
This allows to remove some errno saving outside of talloc.c.
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
tools/xenstore/talloc.c | 25 ++++++++++++++++++-------
tools/xenstore/xenstored_core.c | 2 --
2 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/tools/xenstore/talloc.c b/tools/xenstore/talloc.c
index d7edcf3a93..5fbefdf091 100644
--- a/tools/xenstore/talloc.c
+++ b/tools/xenstore/talloc.c
@@ -103,6 +103,8 @@ struct talloc_chunk {
unsigned flags;
};
+static int _talloc_free(void *ptr);
+
/* 16 byte alignment seems to keep everyone happy */
#define TC_HDR_SIZE ((sizeof(struct talloc_chunk)+15)&~15)
#define TC_PTR_FROM_CHUNK(tc) ((void *)(TC_HDR_SIZE + (char*)tc))
@@ -245,7 +247,7 @@ static int talloc_reference_destructor(void *ptr)
tc1->destructor = NULL;
}
_TLIST_REMOVE(tc2->refs, handle);
- talloc_free(handle);
+ _talloc_free(handle);
From the commit message, it is not clear to me why we are calling the
underscore version here. Same for the others below.
return 0;
}
@@ -311,7 +313,7 @@ static int talloc_unreference(const void *context, const void *ptr)
talloc_set_destructor(h, NULL);
_TLIST_REMOVE(tc->refs, h);
- talloc_free(h);
+ _talloc_free(h);
return 0;
}
@@ -349,7 +351,7 @@ 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(ptr);
}
new_p = talloc_parent_chunk(tc_p->refs);
@@ -521,7 +523,7 @@ static 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 (talloc_free(child) == -1) {
+ if (_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);
@@ -539,7 +541,7 @@ static void talloc_free_children(void *ptr)
will not be freed if the ref_count is > 1 or the destructor (if
any) returns non-zero
Can you expand this comment to explain the different between
_talloc_free() and talloc_free()?
I agree the code is probably clear enough, but better to be obvious.
*/
-int talloc_free(void *ptr)
+static int _talloc_free(void *ptr)
{
struct talloc_chunk *tc;
@@ -597,7 +599,16 @@ int talloc_free(void *ptr)
return 0;
}
+int talloc_free(void *ptr)
+{
+ int ret;
+ int saved_errno = errno;
+ ret = _talloc_free(ptr);
+ errno = saved_errno;
+
+ return ret;
+}
/*
A talloc version of realloc. The context argument is only used if
@@ -610,7 +621,7 @@ void *_talloc_realloc(const void *context, void *ptr,
size_t size, const char *n
/* size zero is equivalent to free() */
if (size == 0) {
- talloc_free(ptr);
+ _talloc_free(ptr);
return NULL;
}
@@ -1243,7 +1254,7 @@ void *talloc_realloc_fn(const void *context, void *ptr, size_t size)
static void talloc_autofree(void)
{
- talloc_free(cleanup_context);
+ _talloc_free(cleanup_context);
cleanup_context = NULL;
}
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 476d5c6d51..5a174b9881 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -771,9 +771,7 @@ struct node *read_node(struct connection *conn, const void
*ctx,
return node;
error:
- err = errno;
talloc_free(node);
- errno = err;
return NULL;
}
Cheers,
--
Julien Grall