Re: [PATCH 03/20] tools/xenstore: let talloc_free() preserve errno

[Juergen Gross <jgross@xxxxxxxx>]

On 06.11.22 22:08, Julien Grall wrote:
> 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 <jgross@xxxxxxxx>
> > ---
> >   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.
I was targeting only talloc_free() calls from xenstored to preserve errno.

I can see your point that we could just do the same for talloc internal
calls, preserving errno in other cases, too.

OTOH the only relevant case would be the call from talloc_unlink() via
talloc_unreference(), which is at least currently no problem regarding
errno.

Do you have any preferences? I'm leaning towards dropping the wrapper
and do the errno preserving just inside talloc_free().


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature