Re: [XEN PATCH] libxl_nocpuid.c: fix build with json-c

[Anthony PERARD <anthony@xenproject.org>]

On Mon, Nov 24, 2025 at 10:26:34AM +0100, Jan Beulich wrote:
> On 21.11.2025 22:09, Michael Young wrote:
> > diff --git a/tools/libs/light/libxl_nocpuid.c 
> > b/tools/libs/light/libxl_nocpuid.c
> > index 0630959e76..71ab49ed61 100644
> > --- a/tools/libs/light/libxl_nocpuid.c
> > +++ b/tools/libs/light/libxl_nocpuid.c
> > @@ -40,11 +40,24 @@ int libxl__cpuid_legacy(libxl_ctx *ctx, uint32_t 
> > domid, bool restore,
> >       return 0;
> >   }
> > 
> > +#ifdef HAVE_LIBJSONC
> > +#ifndef _hidden
> > +#define _hidden
> > +#endif
> 
> Why would this be needed? libxl_internal provides a definition afaics.
> 
> > +_hidden int libxl_cpuid_policy_list_gen_jso(json_object **jso_r,
> 
> Nor should the attribute be needed here, as the function declaration ought
> to be in scope.

Yes, just mirroring the changes done to libxl_cpuid.c should be enough.

> > +                                libxl_cpuid_policy_list *pcpuid)
> > +{
> > +    return 0;
> > +}
> > +#endif
> > +
> > +#if defined(HAVE_LIBYAJL)
> >   yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
> >                                   libxl_cpuid_policy_list *pcpuid)
> >   {
> >       return 0;
> >   }
> > +#endif
> 
> Maybe unrelated to this build fix, I find it hard to believe that returning
> 0 (presumably meaning "success") here is appropriate without actually doing
> anything. In particular for the new function you add, I think upon success
> the caller can expect *jso_r to have got assigned a value. However, without
> any commentary it's hard to tell whether there's some "agreement" that the
> caller has to pre-set its variable (to, say, NULL).

For the YAJL function, this is correct, there's nothing to do with
`hand`. But for the json-c function, while the current caller does
initialise `*jso_r`, it would be best indeed to have the function set
`*jso_r` to NULL. (It's equivalent to return a NULL JSON object.)

> Also why are the libxl_..._jso() all hidden, while their libxl_..._json()
> counterparts aren't? And why would non-exported functions have their
> declarations live in a non-private header?

History, bad demission which expose libxl internals to libxl's users,
which make it impossible to use whatever json parsing/generating library that
libxl wants. The header `libxl_json.h` is only necessary if you want to
use the `*_json()` functions, and the only user (I hope) is `xl`.
I guess we could remove the installation of "libxl_json.h" header (and
"_libxl_types_json.h") when building libxl with json-c support.

Thanks,

-- 
Anthony PERARD