On 05/17/2013 02:19 AM, WangXu wrote:
> ----------------------------------------
>> From: jfer...@redhat.com
>> To: libvirt-cim@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@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvirt-cim
>>
I will squash in the following:
diff --git a/src/Virt_RASD.c b/src/Virt_RASD.c
index a4cba5b..af6a43f 100644
--- a/src/Virt_RASD.c
+++ b/src/Virt_RASD.c
@@ -142,8 +142,14 @@ static CMPIStatus set_proc_rasd_params(const
CMPIBroker *br
if (domain_online(dom)) {
int active_count = domain_vcpu_count(dom);
- if (active_count > 0)
- count = active_count;
+ if (active_count < 0) {
+ cu_statusf(broker, &s,
+ CMPI_RC_ERR_FAILED,
+ "Unable to get domain `%s' vcpu count",
+ domain);
+ goto out;
+ }
+ count = active_count;
} else {
count = dev->dev.vcpu.quantity;
}
NOTE: No need for an else to if (active_count < 0) since we're jumping
to out
John
_______________________________________________
Libvirt-cim mailing list
Libvirt-cim@redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-cim
