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
