Re: [PATCH v3 6/7] tools/libs: add a new libxenmanage library

[Juergen Gross <jgross@xxxxxxxx>]

On 16.12.24 15:49, Anthony PERARD wrote:
> On Fri, Dec 13, 2024 at 05:24:20PM +0100, Juergen Gross wrote:
> > diff --git a/tools/include/xenmanage.h b/tools/include/xenmanage.h
> > new file mode 100644
> > index 0000000000..5d169693b0
> > --- /dev/null
> > +++ b/tools/include/xenmanage.h
> > @@ -0,0 +1,92 @@
> > +/* SPDX-License-Identifier: LGPL-2.1 */
> This should read "LGPL-2.1-only".
Okay.

> > diff --git a/tools/libs/manage/core.c b/tools/libs/manage/core.c
> > new file mode 100644
> > index 0000000000..b5fa67b036
> > --- /dev/null
> > +++ b/tools/libs/manage/core.c
> > @@ -0,0 +1,168 @@
> > +static int xenmanage_do_domctl_get_domain_state(xenmanage_handle *hdl,
> > +                                                unsigned int domid_in,
> > +                                                unsigned int *domid_out,
> > +                                                unsigned int *state,
> > +                                                uint64_t *unique_id)
> > +{
> > +    struct xen_domctl *buf;
> > +    int saved_errno;
> > +    int ret;
> > +
> > +    buf = xencall_alloc_buffer(hdl->xcall, sizeof(*buf));
> > +    if ( !buf )
> > +    {
> > +        errno = ENOMEM;
> Is this necessary? Isn't xencall_alloc_buffer() going to set `errno`? Or
> is it to overwrite `errno` set by system calls which could be other than
> ENOMEM?
There are many different errno values thinkable. I want to make sure
to have a value making clear that the allocation failed for some reason.

> > +        return -1;
> > +    }
> Otherwise, patch looks fine to me: Reviewed-by: Anthony PERARD 
> <anthony.perard@xxxxxxxxxx>
Thanks,


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature