* [XEN PATCH] libxl_nocpuid.c: fix build with json-c
@ 2025-11-21 21:09 Michael Young
2025-11-24 9:26 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: Michael Young @ 2025-11-21 21:09 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Juergen Gross
[-- Attachment #1: Type: text/plain, Size: 1682 bytes --]
The build of xen-4.21.0 with json-c present and yajl not present
is failing with the error
libxl_nocpuid.c:43:1: error: unknown type name ‘yajl_gen_status’
43 | yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
| ^~~~~~~~~~~~~~~
libxl_nocpuid.c:43:50: error: unknown type name ‘yajl_gen’
43 | yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
| ^~~~~~~~
make[6]: ***
[/builddir/build/BUILD/xen-4.21.0-build/xen-4.21.0/tools/libs/light/../../../tools/Rules.mk:178: libxl_nocpuid.o] Error 1
I tested it with the patch below based on libxl_nocpuid.c which did build.
Signed-off-by: Michael Young <m.a.young@durham.ac.uk>
---
tools/libs/light/libxl_nocpuid.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
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
+_hidden int libxl_cpuid_policy_list_gen_jso(json_object **jso_r,
+ 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
int libxl__cpuid_policy_list_parse_json(libxl__gc *gc,
const libxl__json_object *o,
--
2.51.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [XEN PATCH] libxl_nocpuid.c: fix build with json-c 2025-11-21 21:09 [XEN PATCH] libxl_nocpuid.c: fix build with json-c Michael Young @ 2025-11-24 9:26 ` Jan Beulich 2025-11-27 11:05 ` Anthony PERARD 0 siblings, 1 reply; 3+ messages in thread From: Jan Beulich @ 2025-11-24 9:26 UTC (permalink / raw) To: Michael Young, Anthony PERARD; +Cc: Juergen Gross, xen-devel On 21.11.2025 22:09, Michael Young wrote: > The build of xen-4.21.0 with json-c present and yajl not present > is failing with the error > > libxl_nocpuid.c:43:1: error: unknown type name ‘yajl_gen_status’ > 43 | yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand, > | ^~~~~~~~~~~~~~~ > libxl_nocpuid.c:43:50: error: unknown type name ‘yajl_gen’ > 43 | yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand, > | ^~~~~~~~ > make[6]: *** > [/builddir/build/BUILD/xen-4.21.0-build/xen-4.21.0/tools/libs/light/../../../tools/Rules.mk:178: libxl_nocpuid.o] Error 1 > > I tested it with the patch below based on libxl_nocpuid.c which did build. > > Signed-off-by: Michael Young <m.a.young@durham.ac.uk> > --- > tools/libs/light/libxl_nocpuid.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > 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. > + 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). 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? Jan ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [XEN PATCH] libxl_nocpuid.c: fix build with json-c 2025-11-24 9:26 ` Jan Beulich @ 2025-11-27 11:05 ` Anthony PERARD 0 siblings, 0 replies; 3+ messages in thread From: Anthony PERARD @ 2025-11-27 11:05 UTC (permalink / raw) To: Jan Beulich, Michael Young; +Cc: Anthony PERARD, Juergen Gross, xen-devel 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-11-27 11:06 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-21 21:09 [XEN PATCH] libxl_nocpuid.c: fix build with json-c Michael Young 2025-11-24 9:26 ` Jan Beulich 2025-11-27 11:05 ` Anthony PERARD
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.