[libvirt] [PATCH 13/26] qemu: Resolve Coverity FORWARD_NULL

John Ferlan jferlan at redhat.com
Tue Sep 9 11:02:28 UTC 2014



On 09/09/2014 03:36 AM, Wang Rui wrote:
> On 2014/9/5 6:26, John Ferlan wrote:
>> If the virJSONValueNewObject() fails, then rather than going to error
>> and getting a Coverity false positive since it doesn't seem to understand
>> the relationship between nkeywords, keywords, and values and seems to
>> believe calling qemuFreeKeywords will cause a NULL deref - just return NULL
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  src/qemu/qemu_monitor_json.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
>> index 62e7d5d..b0bc5fc 100644
>> --- a/src/qemu/qemu_monitor_json.c
>> +++ b/src/qemu/qemu_monitor_json.c
>> @@ -617,7 +617,7 @@ qemuMonitorJSONKeywordStringToJSON(const char *str, const char *firstkeyword)
>>      size_t i;
>>  
>>      if (!(ret = virJSONValueNewObject()))
>> -        goto error;
>> +        return NULL;
> 
> Maybe it's not enough.
> 
>>      if (qemuParseKeywords(str, &keywords, &values, &nkeywords, 1) < 0)
>>          goto error;
> 
> Here, qemuParseKeywords() may fail and 'values' is still NULL.
> 

Yes, but for some reason once qemuParseKeywords() is called Coverity has
"resolved" that things will be OK.

This one is a false positive and perhaps a bug in the Coverity parser.
The return NULL is the way to avoid the condition. Filing a Coverity bug
is an option, but changing the logic is just quicker.

I think it gets flagged because Coverity perhaps prescans the code and
"flags" all callers to qemuFreeKeywords as potentially having a
FORWARD_NULL. It doesn't seem to pick up the fact that nkeywords is
initialized to 0 (along with keywords and values being set to NULL).
It's perhaps focusing on the array addresses rather than the logic of
using nkeywords to access those arrays.

John




More information about the libvir-list mailing list