[Libvirt-cim] [PATCH 11/19] Coverity: Resolve NO_EFFECT - set_proc_rasd_params()

[WangXu]

----------------------------------------
> From: jferlan at redhat.com
> To: libvirt-cim at redhat.com
> Date: Thu, 16 May 2013 10:57:46 -0400
> Subject: [Libvirt-cim] [PATCH 11/19] Coverity: Resolve NO_EFFECT - set_proc_rasd_params()
>
> 143 if (domain_online(dom))
> 144 count = domain_vcpu_count(dom);
> 145 else
> 146 count = dev->dev.vcpu.quantity;
> 147
>
> (1) Event unsigned_compare:
> This greater-than-or-equal-to-zero comparison of an unsigned value
> is always true. "count>= 0UL".
>
> 148 if (count>= 0)
>
> Resolve by adjusting logic. Problem was that the active count is returned
> as an int with an error value of -1, while the quantity value is guaranteed
> to be 1 or more (see parse_vcpu_device() processing). So initialize count to
> zero, then only set the property if count> 0. Setting count of the active
> condition requires a local "active_count" and checking that to be> 0 before
> blindly setting it to count. Imagine 0xfffffffffffffff vcpu's!
> ---
> src/Virt_RASD.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/src/Virt_RASD.c b/src/Virt_RASD.c
> index ad1a2e7..a4cba5b 100644
> --- a/src/Virt_RASD.c
> +++ b/src/Virt_RASD.c
> @@ -124,7 +124,7 @@ static CMPIStatus set_proc_rasd_params(const CMPIBroker *broker,
> struct infostore_ctx *info = NULL;
> uint32_t weight = 0;
> uint64_t limit;
> - uint64_t count;
> + uint64_t count = 0;
>
> conn = connect_by_classname(broker, CLASSNAME(ref), &s);
> if (conn == NULL)
> @@ -140,12 +140,15 @@ static CMPIStatus set_proc_rasd_params(const CMPIBroker *broker,
> goto out;
> }
>
> - if (domain_online(dom))
> - count = domain_vcpu_count(dom);
> - else
> + if (domain_online(dom)) {
> + int active_count = domain_vcpu_count(dom);
> + if (active_count> 0)
> + count = active_count;
> + } else {
> count = dev->dev.vcpu.quantity;
> + }
if there was some failure happened, domain_vcpu_count would return -1. I think some
code should handle this failure instead of just ignore it and skip CMSetProperty, Such
as cu_statusf and goto out;
My suggestion is the code like this:

if (domain_online(dom)) {
int active_count = domain_vcpu_count(dom);
if (active_count < 0) {  //count never be less than zero and maybe err code -2, -3...would
                         //be added someday.
cu_status(...);
goto out;
} else {
count = active_count;    //equal or more than zero should be OK
}
} else {
count = dev->dev.vcpu.quantity;
}

Xu Wang
>
> - if (count>= 0)
> + if (count> 0)
> CMSetProperty(inst,
> "VirtualQuantity",
> (CMPIValue *)&count,
> --
> 1.8.1.4
>
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim