[libvirt] [PATCH] conf: Resolve Coverity FORWARD_NULL

[Michal Privoznik]

On 04.08.2015 17:54, John Ferlan wrote:
> 
> 
> On 08/04/2015 11:42 AM, Michal Privoznik wrote:
>> On 04.08.2015 13:11, John Ferlan wrote:
>>> The recent changes to perform SCSI device address checks during the
>>> post parse callbacks ran afoul of the Coverity checker since the changes
>>> assumed that the 'xmlopt' parameter to virDomainDeviceDefPostParse
>>> would be non NULL (commit id 'ca2cf74e87'); however, what was missed
>>> is there was an "if (xmlopt &&" check being made, so Coverity believed
>>> that it could be possible for a NULL 'xmlopt'.
>>>
>>> Checking the various calling paths seemingly disproves that. If called
>>> from virDomainDeviceDefParse, there were two other possible calls that
>>> would end up dereffing, so that path could not be NULL. If called via
>>> virDomainDefPostParseDeviceIterator via virDomainDefPostParse there
>>> are two callers (virDomainDefParseXML and qemuParseCommandLine)
>>> which deref xmlopt either directly or through another call.
>>>
>>> So I'm removing the check for non-NULL xmlopt.
>>>
>>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>>> ---
>>>  src/conf/domain_conf.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>> index 77a50c3..dd5ebd7 100644
>>> --- a/src/conf/domain_conf.c
>>> +++ b/src/conf/domain_conf.c
>>> @@ -4145,7 +4145,7 @@ virDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
>>>  {
>>>      int ret;
>>>  
>>> -    if (xmlopt && xmlopt->config.devicesPostParseCallback) {
>>> +    if (xmlopt->config.devicesPostParseCallback) {
>>>          ret = xmlopt->config.devicesPostParseCallback(dev, def, caps,
>>>                                                        xmlopt->config.priv);
>>>          if (ret < 0)
>>>
>>
>> ACK
>>
>> Although with the virDomainDefPostParse() it should be the same story.
>> @xmlopt can't be NULL there too. But that can be saved for a follow up
>> patch if you want.
>>
>> Michal
>>
> 
> Myopia strikes again...  I need a larger workspace!
> 
> John
> 
> Perhaps it'd be better to squash in the following rather than have a
> separate patch since it's same issue even though Coverity only complained
> about the initial one. Although if it's preferred to have a separate
> patch that's fine by me too:
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7d3a24d..5eaeb21 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4190,7 +4190,7 @@ virDomainDefPostParse(virDomainDefPtr def,
>      };
>  
>      /* call the domain config callback */
> -    if (xmlopt && xmlopt->config.domainPostParseCallback) {
> +    if (xmlopt->config.domainPostParseCallback) {
>          ret = xmlopt->config.domainPostParseCallback(def, caps,
>                                                       xmlopt->config.priv);
>          if (ret < 0)
> 
> 

I'd prefer to have it in a single patch. ACK with this squashed in then.

Michal