[PATCH v2 01/19] tools/xenstore: let talloc_free() preserve errno

Juergen Gross posted 19 patches 1 year, 11 months ago
There is a newer version of this series
[PATCH v2 01/19] tools/xenstore: let talloc_free() preserve errno
Posted by Juergen Gross 1 year, 11 months ago
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 <jgross@suse.com>
---
V2:
- drop wrapper (Julien Grall)
---
 tools/xenstore/talloc.c         | 21 +++++++++++++--------
 tools/xenstore/xenstored_core.c |  2 --
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/tools/xenstore/talloc.c b/tools/xenstore/talloc.c
index d7edcf3a93..23c3a23b19 100644
--- a/tools/xenstore/talloc.c
+++ b/tools/xenstore/talloc.c
@@ -541,38 +541,39 @@ static void talloc_free_children(void *ptr)
 */
 int talloc_free(void *ptr)
 {
+	int saved_errno = errno;
 	struct talloc_chunk *tc;
 
 	if (ptr == NULL) {
-		return -1;
+		goto err;
 	}
 
 	tc = talloc_chunk_from_ptr(ptr);
 
 	if (tc->null_refs) {
 		tc->null_refs--;
-		return -1;
+		goto err;
 	}
 
 	if (tc->refs) {
 		talloc_reference_destructor(tc->refs);
-		return -1;
+		goto err;
 	}
 
 	if (tc->flags & TALLOC_FLAG_LOOP) {
 		/* we have a free loop - stop looping */
-		return 0;
+		goto success;
 	}
 
 	if (tc->destructor) {
 		talloc_destructor_t d = tc->destructor;
 		if (d == (talloc_destructor_t)-1) {
-			return -1;
+			goto err;
 		}
 		tc->destructor = (talloc_destructor_t)-1;
 		if (d(ptr) == -1) {
 			tc->destructor = d;
-			return -1;
+			goto err;
 		}
 		tc->destructor = NULL;
 	}
@@ -594,10 +595,14 @@ int talloc_free(void *ptr)
 	tc->flags |= TALLOC_FLAG_FREE;
 
 	free(tc);
+ success:
+	errno = saved_errno;
 	return 0;
-}
-
 
+ err:
+	errno = saved_errno;
+	return -1;
+}
 
 /*
   A talloc version of realloc. The context argument is only used if
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;
 }
 
-- 
2.35.3
Re: [PATCH v2 01/19] tools/xenstore: let talloc_free() preserve errno
Posted by Jan Beulich 1 year, 11 months ago
On 13.12.2022 17:00, 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 looks to be stale ...

> This allows to remove some errno saving outside of talloc.c.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> V2:
> - drop wrapper (Julien Grall)

... after this change?

Jan
Re: [PATCH v2 01/19] tools/xenstore: let talloc_free() preserve errno
Posted by Juergen Gross 1 year, 11 months ago
On 14.12.22 10:54, Jan Beulich wrote:
> On 13.12.2022 17:00, 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 looks to be stale ...
> 
>> This allows to remove some errno saving outside of talloc.c.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> V2:
>> - drop wrapper (Julien Grall)
> 
> ... after this change?

Oh yes, indeed.

It should be replaced by:

"So preserve errno in talloc_free()."

Juergen