[libvirt] [PATCH 04/10] parallels: Need to handle virJSONValueObjectGetString error

[Eric Blake]

On 01/17/2013 01:03 PM, Peter Krempa wrote:
> On 01/17/13 20:17, John Ferlan wrote:
>> ---
>>   src/parallels/parallels_driver.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/parallels/parallels_driver.c
>> b/src/parallels/parallels_driver.c
>> index ea193af..1cf66ea 100644
>> --- a/src/parallels/parallels_driver.c
>> +++ b/src/parallels/parallels_driver.c
>> @@ -514,7 +514,10 @@ parallelsGetNetInfo(virDomainNetDefPtr net,
>>
>>       net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP;
>>       if (virJSONValueObjectHasKey(value, "state")) {
>> -        tmp = virJSONValueObjectGetString(value, "state");
> 
> Hm calling this without checking should be safe as the
> virJSONValueObjectHasKey basically guarantees successful return of a
> value in virJSONValueObjectGetString.
> 
>> +        if (!(tmp = virJSONValueObjectGetString(value, "state"))) {
>> +            parallelsParseError();
>> +            goto error;
>> +        }
>>           if STREQ(tmp, "disconnected")

Another three possible solutions, both shorter, for silencing Coverity
would be:

1. Add in a null check (slight speed penalty, though):
if (virJSONValueObjectHasKey(value, "state")) {
  tmp = virJSONValueObjectGetString(value, "state");
  if STREQ_NULLABLE(tmp, "disconnected")

2. use ga_assert() to tell Coverity something that we already know:
if (virJSONValueObjectHasKey(value, "state")) {
  tmp = virJSONValueObjectGetString(value, "state");
  ga_assert(tmp);
  if STREQ(tmp, "disconnected")

3. My favorite - use fewer function calls to begin with:
if ((tmp = virJSONValueObjectGetString(value, "state")) &&
    STREQ(tmp, "disconnected"))

> I give ACK to this patch, but it shouldn't be a problem to go without it.

I think a v2 would be appropriate with a shorter solution.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130117/8e2dafe2/attachment-0001.sig>