These two pointers can never be NULL since they are initialised to a reference
of a struct. This became apparent when commit 210a19539447 added a VIR_DEBUG
which used both pointers because due to the concise condition the compiler saw
that if the "and" part of the condition did short-circuit (and it assumed that
can happen) the second variable would not be initialised, but it is used in the
debugging message, so the build failed with:
In file included from ../src/cpu/cpu_x86.c:27:
../src/cpu/cpu_x86.c: In function ‘virCPUx86DataIsIdentical’:
../src/util/virlog.h:79:5: error: ‘bdata’ may be used uninitialized in this
function [-Werror=maybe-uninitialized]
Fix this by just assigning the helper pointers and remove the condition
altogether.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
Pushed under the build-breaker rule, but feel free to review/object.
src/cpu/cpu_x86.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 7e9d1cea47d1..a5eac7601cad 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -3314,10 +3314,8 @@ virCPUx86DataIsIdentical(const virCPUData *a,
return VIR_CPU_COMPARE_INCOMPATIBLE;
}
- if (!((adata = &a->data.x86) && (bdata = &b->data.x86))) {
- VIR_DEBUG("missing x86 data: a:%p b:%p", adata, bdata);
- return VIR_CPU_COMPARE_ERROR;
- }
+ adata = &a->data.x86;
+ bdata = &b->data.x86;
if (adata->len != bdata->len) {
VIR_DEBUG("unequal length a:%zu b:%zu", adata->len, bdata->len);
--
2.35.1
On Wed, Apr 27, 2022 at 09:51:34 +0200, Martin Kletzander wrote: > These two pointers can never be NULL since they are initialised to a reference > of a struct. This became apparent when commit 210a19539447 added a VIR_DEBUG > which used both pointers because due to the concise condition the compiler saw > that if the "and" part of the condition did short-circuit (and it assumed that > can happen) the second variable would not be initialised, but it is used in the > debugging message, so the build failed with: > > In file included from ../src/cpu/cpu_x86.c:27: > ../src/cpu/cpu_x86.c: In function ‘virCPUx86DataIsIdentical’: > ../src/util/virlog.h:79:5: error: ‘bdata’ may be used uninitialized in this > function [-Werror=maybe-uninitialized] > > Fix this by just assigning the helper pointers and remove the condition > altogether. > > Signed-off-by: Martin Kletzander <mkletzan@redhat.com> > --- > Pushed under the build-breaker rule, but feel free to review/object. Oops! Unfortunately neither my complilers: clang version 13.0.0 (Fedora 13.0.0-3.fc35), gcc (GCC) 11.2.1 20220127 (Red Hat 11.2.1-9) nor the CI: https://gitlab.com/pipo.sk/libvirt/-/pipelines/524030525 (besides the usual macos-11 fail) complained. Which compiler did you use? > > src/cpu/cpu_x86.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c > index 7e9d1cea47d1..a5eac7601cad 100644 > --- a/src/cpu/cpu_x86.c > +++ b/src/cpu/cpu_x86.c > @@ -3314,10 +3314,8 @@ virCPUx86DataIsIdentical(const virCPUData *a, > return VIR_CPU_COMPARE_INCOMPATIBLE; > } > > - if (!((adata = &a->data.x86) && (bdata = &b->data.x86))) { > - VIR_DEBUG("missing x86 data: a:%p b:%p", adata, bdata); > - return VIR_CPU_COMPARE_ERROR; > - } > + adata = &a->data.x86; > + bdata = &b->data.x86; Yup, checking whether the result of the '&' operator is non-null doesn't make sense.
On Wed, Apr 27, 2022 at 10:12:25AM +0200, Peter Krempa wrote: >On Wed, Apr 27, 2022 at 09:51:34 +0200, Martin Kletzander wrote: >> These two pointers can never be NULL since they are initialised to a reference >> of a struct. This became apparent when commit 210a19539447 added a VIR_DEBUG >> which used both pointers because due to the concise condition the compiler saw >> that if the "and" part of the condition did short-circuit (and it assumed that >> can happen) the second variable would not be initialised, but it is used in the >> debugging message, so the build failed with: >> >> In file included from ../src/cpu/cpu_x86.c:27: >> ../src/cpu/cpu_x86.c: In function ‘virCPUx86DataIsIdentical’: >> ../src/util/virlog.h:79:5: error: ‘bdata’ may be used uninitialized in this >> function [-Werror=maybe-uninitialized] >> >> Fix this by just assigning the helper pointers and remove the condition >> altogether. >> >> Signed-off-by: Martin Kletzander <mkletzan@redhat.com> >> --- >> Pushed under the build-breaker rule, but feel free to review/object. > >Oops! Unfortunately neither my complilers: > > clang version 13.0.0 (Fedora 13.0.0-3.fc35), > gcc (GCC) 11.2.1 20220127 (Red Hat 11.2.1-9) > >nor the CI: > > https://gitlab.com/pipo.sk/libvirt/-/pipelines/524030525 (besides the > usual macos-11 fail) > >complained. Which compiler did you use? > gcc (Gentoo 11.3.0 p4) 11.3.0 > >> >> src/cpu/cpu_x86.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c >> index 7e9d1cea47d1..a5eac7601cad 100644 >> --- a/src/cpu/cpu_x86.c >> +++ b/src/cpu/cpu_x86.c >> @@ -3314,10 +3314,8 @@ virCPUx86DataIsIdentical(const virCPUData *a, >> return VIR_CPU_COMPARE_INCOMPATIBLE; >> } >> >> - if (!((adata = &a->data.x86) && (bdata = &b->data.x86))) { >> - VIR_DEBUG("missing x86 data: a:%p b:%p", adata, bdata); >> - return VIR_CPU_COMPARE_ERROR; >> - } >> + adata = &a->data.x86; >> + bdata = &b->data.x86; > >Yup, checking whether the result of the '&' operator is non-null doesn't >make sense. >
© 2016 - 2024 Red Hat, Inc.