On Fedora 40, the build fails with:
gcc -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wno-unused-but-set-variable -Wno-unused-local-typedefs -Werror -O2 -fomit-frame-pointer -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MP -MF .domain_getinfo_stubs_v1.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fPIC -I/usr/lib64/ocaml -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs/xsd_glue -I/build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include -D__XEN_TOOLS__ -I/build/tools/ocaml/libs\/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include -c -o domain_getinfo_stubs_v1.o domain_getinfo_stubs_v1.c
In file included from domain_getinfo_stubs_v1.c:10:
domain_getinfo_stubs_v1.c: In function 'xsd_glue_failwith':
/usr/lib64/ocaml/caml/memory.h:275:29: error: unused variable 'caml__frame' [-Werror=unused-variable]
275 | struct caml__roots_block *caml__frame = *caml_local_roots_ptr
| ^~~~~~~~~~~
domain_getinfo_stubs_v1.c:48:9: note: in expansion of macro 'CAMLparam0'
48 | CAMLparam0();
| ^~~~~~~~~~
cc1: all warnings being treated as errors
The CAMLparam0 macro is defined in /usr/lib64/ocaml/caml/memory.h:255 as:
#define CAMLparam0() \
struct caml__roots_block** caml_local_roots_ptr = \
(DO_CHECK_CAML_STATE ? Caml_check_caml_state() : (void)0, \
&CAML_LOCAL_ROOTS); \
struct caml__roots_block *caml__frame = *caml_local_roots_ptr
We can't modify the macro. Mark the xsd_glue_failwith() function with
ignore "-Wunused-variable" to prevent gcc from failing the build due
to the unused variable.
Fixes: a6576011a4d2 ("ocaml/libs: Implement a dynamically-loaded plugin for Xenctrl.domain_getinfo")
Signed-off-by: Javi Merino <javi.merino@cloud.com>
---
While it is not ideal, this is the most effective option I have found
that avoids treating warnings as errors. I can add a comment above
the pragma explaining why if we decide that this is the way forward.
.../domain_getinfo_plugin_v1/domain_getinfo_stubs_v1.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/domain_getinfo_stubs_v1.c b/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/domain_getinfo_stubs_v1.c
index 7be386f4d4c2..df2b3c74c727 100644
--- a/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/domain_getinfo_stubs_v1.c
+++ b/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/domain_getinfo_stubs_v1.c
@@ -42,6 +42,8 @@ static struct custom_operations xsd_glue_xenctrl_ops = {
.compare_ext = custom_compare_ext_default, /* Can't compare */
};
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-variable"
static void Noreturn xsd_glue_failwith(
xc_interface *xch, const char *func, unsigned int line)
{
@@ -70,6 +72,7 @@ static void Noreturn xsd_glue_failwith(
caml_raise_with_arg(*caml_named_value("xsg.error_v1"), msg);
}
+#pragma GCC diagnostic pop
#define xsd_glue_failwith(xch) xsd_glue_failwith(xch, __func__, __LINE__)
CAMLprim value stub_xsd_glue_xc_interface_open(value unit)
--
2.46.1On 17/10/2024 5:20 pm, Javi Merino wrote:
> On Fedora 40, the build fails with:
>
> gcc -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wno-unused-but-set-variable -Wno-unused-local-typedefs -Werror -O2 -fomit-frame-pointer -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MP -MF .domain_getinfo_stubs_v1.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fPIC -I/usr/lib64/ocaml -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs/xsd_glue -I/build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include -D__XEN_TOOLS__ -I/build/tools/ocaml/libs\/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include -c -o domain_getinfo_stubs_v1.o domain_getinfo_stubs_v1.c
> In file included from domain_getinfo_stubs_v1.c:10:
> domain_getinfo_stubs_v1.c: In function 'xsd_glue_failwith':
> /usr/lib64/ocaml/caml/memory.h:275:29: error: unused variable 'caml__frame' [-Werror=unused-variable]
> 275 | struct caml__roots_block *caml__frame = *caml_local_roots_ptr
> | ^~~~~~~~~~~
> domain_getinfo_stubs_v1.c:48:9: note: in expansion of macro 'CAMLparam0'
> 48 | CAMLparam0();
> | ^~~~~~~~~~
> cc1: all warnings being treated as errors
>
> The CAMLparam0 macro is defined in /usr/lib64/ocaml/caml/memory.h:255 as:
>
> #define CAMLparam0() \
> struct caml__roots_block** caml_local_roots_ptr = \
> (DO_CHECK_CAML_STATE ? Caml_check_caml_state() : (void)0, \
> &CAML_LOCAL_ROOTS); \
> struct caml__roots_block *caml__frame = *caml_local_roots_ptr
>
> We can't modify the macro. Mark the xsd_glue_failwith() function with
> ignore "-Wunused-variable" to prevent gcc from failing the build due
> to the unused variable.
>
> Fixes: a6576011a4d2 ("ocaml/libs: Implement a dynamically-loaded plugin for Xenctrl.domain_getinfo")
> Signed-off-by: Javi Merino <javi.merino@cloud.com>
That's horrible...
I note the Ocaml manual even says:
"Some C compilers give bogus warnings about unused variables
caml__dummy_xxx at each use of CAMLparam and CAMLlocal. You should
ignore them." which a brave claim to make...
The problem with pragma gcc is that we support Clang too.
Having had a play, this works too
@@ -45,7 +45,9 @@ static struct custom_operations xsd_glue_xenctrl_ops = {
static void Noreturn xsd_glue_failwith(
xc_interface *xch, const char *func, unsigned int line)
{
+#define caml__frame __attribute__((unused)) caml__frame
CAMLparam0();
+#undef caml__frame
CAMLlocal1(msg);
const xc_error *error = xch ? xc_get_last_error(xch) : NULL;
char *str = NULL;
and is rather more selective.
However, looking through other bits of memory.h, there's this gem:
#define CAMLnoreturn ((void) caml__frame)
which has existed since db3745919 "suppression des warnings "unused
variable" de gcc" in 2004.
So, I think this is a better fix:
@@ -69,6 +69,7 @@ static void Noreturn xsd_glue_failwith(
free(str);
caml_raise_with_arg(*caml_named_value("xsg.error_v1"), msg);
+ CAMLnoreturn;
}
#define xsd_glue_failwith(xch) xsd_glue_failwith(xch, __func__, __LINE__)
and F40 seem happy with the result.
~Andrew
On Thu, Oct 17, 2024 at 06:47:44PM +0100, Andrew Cooper wrote:
> On 17/10/2024 5:20 pm, Javi Merino wrote:
> > On Fedora 40, the build fails with:
> >
> > gcc -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wno-unused-but-set-variable -Wno-unused-local-typedefs -Werror -O2 -fomit-frame-pointer -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MP -MF .domain_getinfo_stubs_v1.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fPIC -I/usr/lib64/ocaml -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs/xsd_glue -I/build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include -D__XEN_TOOLS__ -I/build/tools/ocaml/libs\/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include -c -o domain_getinfo_stubs_v1.o domain_getinfo_stubs_v1.c
> > In file included from domain_getinfo_stubs_v1.c:10:
> > domain_getinfo_stubs_v1.c: In function 'xsd_glue_failwith':
> > /usr/lib64/ocaml/caml/memory.h:275:29: error: unused variable 'caml__frame' [-Werror=unused-variable]
> > 275 | struct caml__roots_block *caml__frame = *caml_local_roots_ptr
> > | ^~~~~~~~~~~
> > domain_getinfo_stubs_v1.c:48:9: note: in expansion of macro 'CAMLparam0'
> > 48 | CAMLparam0();
> > | ^~~~~~~~~~
> > cc1: all warnings being treated as errors
> >
> > The CAMLparam0 macro is defined in /usr/lib64/ocaml/caml/memory.h:255 as:
> >
> > #define CAMLparam0() \
> > struct caml__roots_block** caml_local_roots_ptr = \
> > (DO_CHECK_CAML_STATE ? Caml_check_caml_state() : (void)0, \
> > &CAML_LOCAL_ROOTS); \
> > struct caml__roots_block *caml__frame = *caml_local_roots_ptr
> >
> > We can't modify the macro. Mark the xsd_glue_failwith() function with
> > ignore "-Wunused-variable" to prevent gcc from failing the build due
> > to the unused variable.
> >
> > Fixes: a6576011a4d2 ("ocaml/libs: Implement a dynamically-loaded plugin for Xenctrl.domain_getinfo")
> > Signed-off-by: Javi Merino <javi.merino@cloud.com>
>
> That's horrible...
>
> I note the Ocaml manual even says:
>
> "Some C compilers give bogus warnings about unused variables
> caml__dummy_xxx at each use of CAMLparam and CAMLlocal. You should
> ignore them." which a brave claim to make...
>
>
> The problem with pragma gcc is that we support Clang too.
>
> Having had a play, this works too
>
> @@ -45,7 +45,9 @@ static struct custom_operations xsd_glue_xenctrl_ops = {
> static void Noreturn xsd_glue_failwith(
> xc_interface *xch, const char *func, unsigned int line)
> {
> +#define caml__frame __attribute__((unused)) caml__frame
> CAMLparam0();
> +#undef caml__frame
> CAMLlocal1(msg);
> const xc_error *error = xch ? xc_get_last_error(xch) : NULL;
> char *str = NULL;
>
> and is rather more selective.
>
>
> However, looking through other bits of memory.h, there's this gem:
>
> #define CAMLnoreturn ((void) caml__frame)
>
> which has existed since db3745919 "suppression des warnings "unused
> variable" de gcc" in 2004.
>
> So, I think this is a better fix:
>
> @@ -69,6 +69,7 @@ static void Noreturn xsd_glue_failwith(
> free(str);
>
> caml_raise_with_arg(*caml_named_value("xsg.error_v1"), msg);
> + CAMLnoreturn;
> }
> #define xsd_glue_failwith(xch) xsd_glue_failwith(xch, __func__, __LINE__)
>
Right! I actually tried put a (void)caml__frame to silence the
warning, but I didn't think of looking for a macro that would do
exactly that.
Do I have to resend the series with your patch, or can you commit it
directly?
Cheers,
Javi
On 18/10/2024 9:50 am, Javi Merino wrote:
> On Thu, Oct 17, 2024 at 06:47:44PM +0100, Andrew Cooper wrote:
>> On 17/10/2024 5:20 pm, Javi Merino wrote:
>>> On Fedora 40, the build fails with:
>>>
>>> gcc -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wno-unused-but-set-variable -Wno-unused-local-typedefs -Werror -O2 -fomit-frame-pointer -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MP -MF .domain_getinfo_stubs_v1.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fPIC -I/usr/lib64/ocaml -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs/xsd_glue -I/build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include -D__XEN_TOOLS__ -I/build/tools/ocaml/libs\/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include -c -o domain_getinfo_stubs_v1.o domain_getinfo_stubs_v1.c
>>> In file included from domain_getinfo_stubs_v1.c:10:
>>> domain_getinfo_stubs_v1.c: In function 'xsd_glue_failwith':
>>> /usr/lib64/ocaml/caml/memory.h:275:29: error: unused variable 'caml__frame' [-Werror=unused-variable]
>>> 275 | struct caml__roots_block *caml__frame = *caml_local_roots_ptr
>>> | ^~~~~~~~~~~
>>> domain_getinfo_stubs_v1.c:48:9: note: in expansion of macro 'CAMLparam0'
>>> 48 | CAMLparam0();
>>> | ^~~~~~~~~~
>>> cc1: all warnings being treated as errors
>>>
>>> The CAMLparam0 macro is defined in /usr/lib64/ocaml/caml/memory.h:255 as:
>>>
>>> #define CAMLparam0() \
>>> struct caml__roots_block** caml_local_roots_ptr = \
>>> (DO_CHECK_CAML_STATE ? Caml_check_caml_state() : (void)0, \
>>> &CAML_LOCAL_ROOTS); \
>>> struct caml__roots_block *caml__frame = *caml_local_roots_ptr
>>>
>>> We can't modify the macro. Mark the xsd_glue_failwith() function with
>>> ignore "-Wunused-variable" to prevent gcc from failing the build due
>>> to the unused variable.
>>>
>>> Fixes: a6576011a4d2 ("ocaml/libs: Implement a dynamically-loaded plugin for Xenctrl.domain_getinfo")
>>> Signed-off-by: Javi Merino <javi.merino@cloud.com>
>> That's horrible...
>>
>> I note the Ocaml manual even says:
>>
>> "Some C compilers give bogus warnings about unused variables
>> caml__dummy_xxx at each use of CAMLparam and CAMLlocal. You should
>> ignore them." which a brave claim to make...
>>
>>
>> The problem with pragma gcc is that we support Clang too.
>>
>> Having had a play, this works too
>>
>> @@ -45,7 +45,9 @@ static struct custom_operations xsd_glue_xenctrl_ops = {
>> static void Noreturn xsd_glue_failwith(
>> xc_interface *xch, const char *func, unsigned int line)
>> {
>> +#define caml__frame __attribute__((unused)) caml__frame
>> CAMLparam0();
>> +#undef caml__frame
>> CAMLlocal1(msg);
>> const xc_error *error = xch ? xc_get_last_error(xch) : NULL;
>> char *str = NULL;
>>
>> and is rather more selective.
>>
>>
>> However, looking through other bits of memory.h, there's this gem:
>>
>> #define CAMLnoreturn ((void) caml__frame)
>>
>> which has existed since db3745919 "suppression des warnings "unused
>> variable" de gcc" in 2004.
>>
>> So, I think this is a better fix:
>>
>> @@ -69,6 +69,7 @@ static void Noreturn xsd_glue_failwith(
>> free(str);
>>
>> caml_raise_with_arg(*caml_named_value("xsg.error_v1"), msg);
>> + CAMLnoreturn;
>> }
>> #define xsd_glue_failwith(xch) xsd_glue_failwith(xch, __func__, __LINE__)
>>
> Right! I actually tried put a (void)caml__frame to silence the
> warning, but I didn't think of looking for a macro that would do
> exactly that.
:) It's Ocaml. I had a hunch there'd be something.
> Do I have to resend the series with your patch, or can you commit it
> directly?
I'll just fold it on commit. No point spending more time on this.
In the meantime, Ocaml upstream have already merged my docs PR about this.
~Andrew
> On 17 Oct 2024, at 18:47, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
>
> So, I think this is a better fix:
>
> @@ -69,6 +69,7 @@ static void Noreturn xsd_glue_failwith(
> free(str);
>
> caml_raise_with_arg(*caml_named_value("xsg.error_v1"), msg);
> + CAMLnoreturn;
> }
> #define xsd_glue_failwith(xch) xsd_glue_failwith(xch, __func__, __LINE__)
>
> and F40 seem happy with the result.
Acked-by: Christian Lindig <christian.lindig@cloud.com>
I like the effort to remove warnings.
— C
On 17/10/2024 6:47 pm, Andrew Cooper wrote:
> On 17/10/2024 5:20 pm, Javi Merino wrote:
>> On Fedora 40, the build fails with:
>>
>> gcc -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wno-unused-but-set-variable -Wno-unused-local-typedefs -Werror -O2 -fomit-frame-pointer -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MP -MF .domain_getinfo_stubs_v1.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fPIC -I/usr/lib64/ocaml -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs/xsd_glue -I/build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include -D__XEN_TOOLS__ -I/build/tools/ocaml/libs\/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include -c -o domain_getinfo_stubs_v1.o domain_getinfo_stubs_v1.c
>> In file included from domain_getinfo_stubs_v1.c:10:
>> domain_getinfo_stubs_v1.c: In function 'xsd_glue_failwith':
>> /usr/lib64/ocaml/caml/memory.h:275:29: error: unused variable 'caml__frame' [-Werror=unused-variable]
>> 275 | struct caml__roots_block *caml__frame = *caml_local_roots_ptr
>> | ^~~~~~~~~~~
>> domain_getinfo_stubs_v1.c:48:9: note: in expansion of macro 'CAMLparam0'
>> 48 | CAMLparam0();
>> | ^~~~~~~~~~
>> cc1: all warnings being treated as errors
>>
>> The CAMLparam0 macro is defined in /usr/lib64/ocaml/caml/memory.h:255 as:
>>
>> #define CAMLparam0() \
>> struct caml__roots_block** caml_local_roots_ptr = \
>> (DO_CHECK_CAML_STATE ? Caml_check_caml_state() : (void)0, \
>> &CAML_LOCAL_ROOTS); \
>> struct caml__roots_block *caml__frame = *caml_local_roots_ptr
>>
>> We can't modify the macro. Mark the xsd_glue_failwith() function with
>> ignore "-Wunused-variable" to prevent gcc from failing the build due
>> to the unused variable.
>>
>> Fixes: a6576011a4d2 ("ocaml/libs: Implement a dynamically-loaded plugin for Xenctrl.domain_getinfo")
>> Signed-off-by: Javi Merino <javi.merino@cloud.com>
> That's horrible...
>
> I note the Ocaml manual even says:
>
> "Some C compilers give bogus warnings about unused variables
> caml__dummy_xxx at each use of CAMLparam and CAMLlocal. You should
> ignore them." which a brave claim to make...
>
>
> The problem with pragma gcc is that we support Clang too.
>
> Having had a play, this works too
>
> @@ -45,7 +45,9 @@ static struct custom_operations xsd_glue_xenctrl_ops = {
> static void Noreturn xsd_glue_failwith(
> xc_interface *xch, const char *func, unsigned int line)
> {
> +#define caml__frame __attribute__((unused)) caml__frame
> CAMLparam0();
> +#undef caml__frame
> CAMLlocal1(msg);
> const xc_error *error = xch ? xc_get_last_error(xch) : NULL;
> char *str = NULL;
>
> and is rather more selective.
>
>
> However, looking through other bits of memory.h, there's this gem:
>
> #define CAMLnoreturn ((void) caml__frame)
>
> which has existed since db3745919 "suppression des warnings "unused
> variable" de gcc" in 2004.
>
> So, I think this is a better fix:
>
> @@ -69,6 +69,7 @@ static void Noreturn xsd_glue_failwith(
> free(str);
>
> caml_raise_with_arg(*caml_named_value("xsg.error_v1"), msg);
> + CAMLnoreturn;
> }
> #define xsd_glue_failwith(xch) xsd_glue_failwith(xch, __func__, __LINE__)
>
> and F40 seem happy with the result.
And https://github.com/ocaml/ocaml/pull/13561 to update Ocaml's
documentation on the matter.
~Andrew
© 2016 - 2026 Red Hat, Inc.