Re: [libvirt] [PATCH] qemu: Fix compilation error when enum variable size differs from 'int'
On Mon, May 25, 2015 at 18:28:42 +0200, Michal Privoznik wrote:
> On 25.05.2015 17:47, Peter Krempa wrote:
> > On Mon, May 25, 2015 at 17:35:19 +0200, Michal Privoznik wrote:
> >> On 25.05.2015 17:18, Peter Krempa wrote:
> >>> Since commit bcd9a564b631aa virDomainNumatuneGetMode returns the value
> >>> via a pointer rather than in the return value. The change triggered
> >>> problems with platforms where the compiler decides to use a data type of
> >>> size different than integer at the point where we typecast it.
> >>>
> >>> Work around the issue by using an intermediate variable of the correct
> >>> type that gets casted back by the default typecasting rules.
> >>> ---
> >>> src/qemu/qemu_driver.c | 10 ++
> >>> 1 file changed, 6 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> >>> index aa0acde..2b9f125 100644
> >>> --- a/src/qemu/qemu_driver.c
> >>> +++ b/src/qemu/qemu_driver.c
> >>> @@ -10566,14 +10566,16 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
> >>> virMemoryParameterPtr param = ¶ms[i];
> >>>
> >>> switch (i) {
> >>> -case 0: /* fill numa mode here */
> >>> +case 0: { /* fill numa mode here */
> >>> +virDomainNumatuneMemMode tmp;
> >>> +virDomainNumatuneGetMode(def->numa, -1, &tmp);
> >>> +
> >>> if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
> >>> -VIR_TYPED_PARAM_INT, 0) < 0)
> >>> +VIR_TYPED_PARAM_INT, tmp) < 0)
> >>> goto cleanup;
> >>>
> >>> -virDomainNumatuneGetMode(def->numa, -1,
> >>> - (virDomainNumatuneMemMode *)
> >>> ¶m->value.i);
> >>> break;
> >>> +}
> >>>
> >>> case 1: /* fill numa nodeset here */
> >>> nodeset = virDomainNumatuneFormatNodeset(def->numa, NULL,
> >>> -1);
> >>>
> >>
> >> I guess we saw the same coverity report since you're fixing the code I'm
> >> just looking at. But I think what is coverity trying to tell us is that
> >> in all other places virDomainNumatuneGetMode() is checked for return
> >> value, here it's not. Or did you really see problem you're describing
> >> in the commit message?
> >
> > The problem I'm trying to solve is failure to compile libvirt on CentOS
> > 5. The compilation error can be seen at:
> >
> > https://ci.centos.org/view/libvirt-project/job/libvirt-daemon-build/systems=libvirt-centos-5/259/console
> >
> > if you scroll down enough.
> >
>
> Ah, okay; So ACK. Lets see what will Coverity think after you push this.
Ah, I see what coverity might complain about. The value will not be
initialized if neither the specific nor the default mode is specified in
the XML. I'll send a v2 fixing both the coverity issue and the
compilation problem since this one would not fix coverity as it would be
possible to have @tmp uninitialized.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix compilation error when enum variable size differs from 'int'
On 25.05.2015 17:47, Peter Krempa wrote:
> On Mon, May 25, 2015 at 17:35:19 +0200, Michal Privoznik wrote:
>> On 25.05.2015 17:18, Peter Krempa wrote:
>>> Since commit bcd9a564b631aa virDomainNumatuneGetMode returns the value
>>> via a pointer rather than in the return value. The change triggered
>>> problems with platforms where the compiler decides to use a data type of
>>> size different than integer at the point where we typecast it.
>>>
>>> Work around the issue by using an intermediate variable of the correct
>>> type that gets casted back by the default typecasting rules.
>>> ---
>>> src/qemu/qemu_driver.c | 10 ++
>>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>>> index aa0acde..2b9f125 100644
>>> --- a/src/qemu/qemu_driver.c
>>> +++ b/src/qemu/qemu_driver.c
>>> @@ -10566,14 +10566,16 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
>>> virMemoryParameterPtr param = ¶ms[i];
>>>
>>> switch (i) {
>>> -case 0: /* fill numa mode here */
>>> +case 0: { /* fill numa mode here */
>>> +virDomainNumatuneMemMode tmp;
>>> +virDomainNumatuneGetMode(def->numa, -1, &tmp);
>>> +
>>> if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
>>> -VIR_TYPED_PARAM_INT, 0) < 0)
>>> +VIR_TYPED_PARAM_INT, tmp) < 0)
>>> goto cleanup;
>>>
>>> -virDomainNumatuneGetMode(def->numa, -1,
>>> - (virDomainNumatuneMemMode *)
>>> ¶m->value.i);
>>> break;
>>> +}
>>>
>>> case 1: /* fill numa nodeset here */
>>> nodeset = virDomainNumatuneFormatNodeset(def->numa, NULL, -1);
>>>
>>
>> I guess we saw the same coverity report since you're fixing the code I'm
>> just looking at. But I think what is coverity trying to tell us is that
>> in all other places virDomainNumatuneGetMode() is checked for return
>> value, here it's not. Or did you really see problem you're describing
>> in the commit message?
>
> The problem I'm trying to solve is failure to compile libvirt on CentOS
> 5. The compilation error can be seen at:
>
> https://ci.centos.org/view/libvirt-project/job/libvirt-daemon-build/systems=libvirt-centos-5/259/console
>
> if you scroll down enough.
>
Ah, okay; So ACK. Lets see what will Coverity think after you push this.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix compilation error when enum variable size differs from 'int'
On Mon, May 25, 2015 at 05:35:19PM +0200, Michal Privoznik wrote:
On 25.05.2015 17:18, Peter Krempa wrote:
Since commit bcd9a564b631aa virDomainNumatuneGetMode returns the value
via a pointer rather than in the return value. The change triggered
problems with platforms where the compiler decides to use a data type of
size different than integer at the point where we typecast it.
Work around the issue by using an intermediate variable of the correct
type that gets casted back by the default typecasting rules.
---
src/qemu/qemu_driver.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index aa0acde..2b9f125 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10566,14 +10566,16 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
virMemoryParameterPtr param = ¶ms[i];
switch (i) {
-case 0: /* fill numa mode here */
+case 0: { /* fill numa mode here */
+virDomainNumatuneMemMode tmp;
+virDomainNumatuneGetMode(def->numa, -1, &tmp);
+
if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
-VIR_TYPED_PARAM_INT, 0) < 0)
+VIR_TYPED_PARAM_INT, tmp) < 0)
goto cleanup;
-virDomainNumatuneGetMode(def->numa, -1,
- (virDomainNumatuneMemMode *)
¶m->value.i);
break;
+}
case 1: /* fill numa nodeset here */
nodeset = virDomainNumatuneFormatNodeset(def->numa, NULL, -1);
I guess we saw the same coverity report since you're fixing the code I'm
just looking at. But I think what is coverity trying to tell us is that
in all other places virDomainNumatuneGetMode() is checked for return
value, here it's not. Or did you really see problem you're describing
in the commit message?
I think Peter is trying to fix the problem of compilation on centos 5:
https://ci.centos.org/view/libvirt-project/job/libvirt-daemon-build/systems=libvirt-centos-5/259/console
And I already had similar patch prepared, I only declared the "tmp" as
"memMode" for the whole function because switch cases with curly
brackets -- Yuck!
I'd say ACK, but I'm not sure whether Michal was trying to point out
something more that I misunderstood, so I leave it up to him.
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix compilation error when enum variable size differs from 'int'
On Mon, May 25, 2015 at 17:35:19 +0200, Michal Privoznik wrote:
> On 25.05.2015 17:18, Peter Krempa wrote:
> > Since commit bcd9a564b631aa virDomainNumatuneGetMode returns the value
> > via a pointer rather than in the return value. The change triggered
> > problems with platforms where the compiler decides to use a data type of
> > size different than integer at the point where we typecast it.
> >
> > Work around the issue by using an intermediate variable of the correct
> > type that gets casted back by the default typecasting rules.
> > ---
> > src/qemu/qemu_driver.c | 10 ++
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index aa0acde..2b9f125 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -10566,14 +10566,16 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
> > virMemoryParameterPtr param = ¶ms[i];
> >
> > switch (i) {
> > -case 0: /* fill numa mode here */
> > +case 0: { /* fill numa mode here */
> > +virDomainNumatuneMemMode tmp;
> > +virDomainNumatuneGetMode(def->numa, -1, &tmp);
> > +
> > if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
> > -VIR_TYPED_PARAM_INT, 0) < 0)
> > +VIR_TYPED_PARAM_INT, tmp) < 0)
> > goto cleanup;
> >
> > -virDomainNumatuneGetMode(def->numa, -1,
> > - (virDomainNumatuneMemMode *)
> > ¶m->value.i);
> > break;
> > +}
> >
> > case 1: /* fill numa nodeset here */
> > nodeset = virDomainNumatuneFormatNodeset(def->numa, NULL, -1);
> >
>
> I guess we saw the same coverity report since you're fixing the code I'm
> just looking at. But I think what is coverity trying to tell us is that
> in all other places virDomainNumatuneGetMode() is checked for return
> value, here it's not. Or did you really see problem you're describing
> in the commit message?
The problem I'm trying to solve is failure to compile libvirt on CentOS
5. The compilation error can be seen at:
https://ci.centos.org/view/libvirt-project/job/libvirt-daemon-build/systems=libvirt-centos-5/259/console
if you scroll down enough.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix compilation error when enum variable size differs from 'int'
On 25.05.2015 17:18, Peter Krempa wrote:
> Since commit bcd9a564b631aa virDomainNumatuneGetMode returns the value
> via a pointer rather than in the return value. The change triggered
> problems with platforms where the compiler decides to use a data type of
> size different than integer at the point where we typecast it.
>
> Work around the issue by using an intermediate variable of the correct
> type that gets casted back by the default typecasting rules.
> ---
> src/qemu/qemu_driver.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index aa0acde..2b9f125 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -10566,14 +10566,16 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
> virMemoryParameterPtr param = ¶ms[i];
>
> switch (i) {
> -case 0: /* fill numa mode here */
> +case 0: { /* fill numa mode here */
> +virDomainNumatuneMemMode tmp;
> +virDomainNumatuneGetMode(def->numa, -1, &tmp);
> +
> if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
> -VIR_TYPED_PARAM_INT, 0) < 0)
> +VIR_TYPED_PARAM_INT, tmp) < 0)
> goto cleanup;
>
> -virDomainNumatuneGetMode(def->numa, -1,
> - (virDomainNumatuneMemMode *)
> ¶m->value.i);
> break;
> +}
>
> case 1: /* fill numa nodeset here */
> nodeset = virDomainNumatuneFormatNodeset(def->numa, NULL, -1);
>
I guess we saw the same coverity report since you're fixing the code I'm
just looking at. But I think what is coverity trying to tell us is that
in all other places virDomainNumatuneGetMode() is checked for return
value, here it's not. Or did you really see problem you're describing
in the commit message?
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Fix compilation error when enum variable size differs from 'int'
Since commit bcd9a564b631aa virDomainNumatuneGetMode returns the value
via a pointer rather than in the return value. The change triggered
problems with platforms where the compiler decides to use a data type of
size different than integer at the point where we typecast it.
Work around the issue by using an intermediate variable of the correct
type that gets casted back by the default typecasting rules.
---
src/qemu/qemu_driver.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index aa0acde..2b9f125 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10566,14 +10566,16 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
virMemoryParameterPtr param = ¶ms[i];
switch (i) {
-case 0: /* fill numa mode here */
+case 0: { /* fill numa mode here */
+virDomainNumatuneMemMode tmp;
+virDomainNumatuneGetMode(def->numa, -1, &tmp);
+
if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
-VIR_TYPED_PARAM_INT, 0) < 0)
+VIR_TYPED_PARAM_INT, tmp) < 0)
goto cleanup;
-virDomainNumatuneGetMode(def->numa, -1,
- (virDomainNumatuneMemMode *)
¶m->value.i);
break;
+}
case 1: /* fill numa nodeset here */
nodeset = virDomainNumatuneFormatNodeset(def->numa, NULL, -1);
--
2.4.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
