Hi Juergen,
On 17/01/2023 09:11, Juergen Gross wrote:
> Today talloc_free() is not guaranteed to preserve errno, especially in
> case a custom destructor is being used.
>
> So preserve errno in talloc_free().
>
> This allows to remove some errno saving outside of talloc.c.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Cheers,
> ---
> 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 78a3edaa4e..1650821922 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;
> }
>
--
Julien Grall