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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list