| Message ID | 20211027074542.2607865-1-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qtest: fix 'expression is always false' build failure in qtest_has_accel() | expand |
On Wed, Oct 27, 2021 at 03:45:42AM -0400, Igor Mammedov wrote: > If KVM is disabled or not present, qtest library build > may fail with: > libqtest.c: In function 'qtest_has_accel': > comparison of unsigned expression < 0 is always false > [-Werror=type-limits] > for (i = 0; i < ARRAY_SIZE(targets); i++) { > > due to empty 'targets' array. > Fix it by compiling KVM related part only if > CONFIG_KVM_TARGETS is set. > > Fixes: e741aff0f43343 ("tests: qtest: add qtest_has_accel() to check if tested binary supports accelerator") > Reported-by: Jason Andryuk <jandryuk@gmail.com> > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > tests/qtest/libqtest.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c > index 25aeea385b..9833e16f84 100644 > --- a/tests/qtest/libqtest.c > +++ b/tests/qtest/libqtest.c > @@ -931,6 +931,7 @@ bool qtest_has_accel(const char *accel_name) > return false; > #endif > } else if (g_str_equal(accel_name, "kvm")) { > +#if defined(CONFIG_KVM_TARGETS) Ugh. const char *targets[] = { CONFIG_KVM_TARGETS }; are you use CONFIG_KVM_TARGETS is not defined? Looks like it's defined, just empty. which is not standard C BTW. How about we just add an empty string in meson? diff --git a/meson.build b/meson.build index 2c5b53cbe2..ff85e9c2af 100644 --- a/meson.build +++ b/meson.build @@ -75,7 +75,7 @@ else kvm_targets = [] endif -kvm_targets_c = '' +kvm_targets_c = '""' if not get_option('kvm').disabled() and targetos == 'linux' kvm_targets_c = '"' + '" ,"'.join(kvm_targets) + '"' endif > int i; > const char *arch = qtest_get_arch(); > const char *targets[] = { CONFIG_KVM_TARGETS }; > @@ -942,6 +943,9 @@ bool qtest_has_accel(const char *accel_name) > } > } > } > +#else > + return false; > +#endif > } else { > /* not implemented */ > g_assert_not_reached(); > -- > 2.27.0
On Wed, Oct 27, 2021 at 4:34 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > On Wed, Oct 27, 2021 at 03:45:42AM -0400, Igor Mammedov wrote: > > If KVM is disabled or not present, qtest library build > > may fail with: > > libqtest.c: In function 'qtest_has_accel': > > comparison of unsigned expression < 0 is always false > > [-Werror=type-limits] > > for (i = 0; i < ARRAY_SIZE(targets); i++) { > > > > due to empty 'targets' array. > > Fix it by compiling KVM related part only if > > CONFIG_KVM_TARGETS is set. > > > > Fixes: e741aff0f43343 ("tests: qtest: add qtest_has_accel() to check if tested binary supports accelerator") > > Reported-by: Jason Andryuk <jandryuk@gmail.com> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > --- > > tests/qtest/libqtest.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c > > index 25aeea385b..9833e16f84 100644 > > --- a/tests/qtest/libqtest.c > > +++ b/tests/qtest/libqtest.c > > @@ -931,6 +931,7 @@ bool qtest_has_accel(const char *accel_name) > > return false; > > #endif > > } else if (g_str_equal(accel_name, "kvm")) { > > +#if defined(CONFIG_KVM_TARGETS) > > Ugh. > const char *targets[] = { CONFIG_KVM_TARGETS }; > > are you use CONFIG_KVM_TARGETS is not defined? > > Looks like it's defined, just empty. > > which is not standard C BTW. > > How about we just add an empty string in meson? > > diff --git a/meson.build b/meson.build > index 2c5b53cbe2..ff85e9c2af 100644 > --- a/meson.build > +++ b/meson.build > @@ -75,7 +75,7 @@ else > kvm_targets = [] > endif > > -kvm_targets_c = '' > +kvm_targets_c = '""' > if not get_option('kvm').disabled() and targetos == 'linux' > kvm_targets_c = '"' + '" ,"'.join(kvm_targets) + '"' > endif This allows Debian buster (gcc 8.3.0) to compile. Thanks, Jason
On 27/10/21 09:45, Igor Mammedov wrote: > If KVM is disabled or not present, qtest library build > may fail with: > libqtest.c: In function 'qtest_has_accel': > comparison of unsigned expression < 0 is always false > [-Werror=type-limits] > for (i = 0; i < ARRAY_SIZE(targets); i++) { > > due to empty 'targets' array. > Fix it by compiling KVM related part only if > CONFIG_KVM_TARGETS is set. Another possibility is to add a trailing NULL to the array and change the loop to "for (i = 0; targets[i]; i++)". Paolo
On Wed, 27 Oct 2021 17:59:44 +0200 Paolo Bonzini <pbonzini@redhat.com> wrote: > On 27/10/21 09:45, Igor Mammedov wrote: > > If KVM is disabled or not present, qtest library build > > may fail with: > > libqtest.c: In function 'qtest_has_accel': > > comparison of unsigned expression < 0 is always false > > [-Werror=type-limits] > > for (i = 0; i < ARRAY_SIZE(targets); i++) { > > > > due to empty 'targets' array. > > Fix it by compiling KVM related part only if > > CONFIG_KVM_TARGETS is set. > > Another possibility is to add a trailing NULL to the array and change > the loop to "for (i = 0; targets[i]; i++)". Thanks, I've sent v2 with Michael suggested approach, it seems to me a bit more cleaner. > > Paolo >
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c index 25aeea385b..9833e16f84 100644 --- a/tests/qtest/libqtest.c +++ b/tests/qtest/libqtest.c @@ -931,6 +931,7 @@ bool qtest_has_accel(const char *accel_name) return false; #endif } else if (g_str_equal(accel_name, "kvm")) { +#if defined(CONFIG_KVM_TARGETS) int i; const char *arch = qtest_get_arch(); const char *targets[] = { CONFIG_KVM_TARGETS }; @@ -942,6 +943,9 @@ bool qtest_has_accel(const char *accel_name) } } } +#else + return false; +#endif } else { /* not implemented */ g_assert_not_reached();
If KVM is disabled or not present, qtest library build may fail with: libqtest.c: In function 'qtest_has_accel': comparison of unsigned expression < 0 is always false [-Werror=type-limits] for (i = 0; i < ARRAY_SIZE(targets); i++) { due to empty 'targets' array. Fix it by compiling KVM related part only if CONFIG_KVM_TARGETS is set. Fixes: e741aff0f43343 ("tests: qtest: add qtest_has_accel() to check if tested binary supports accelerator") Reported-by: Jason Andryuk <jandryuk@gmail.com> Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- tests/qtest/libqtest.c | 4 ++++ 1 file changed, 4 insertions(+)