[libvirt] [PATCH] conf: Resolve Coverity FORWARD_NULL

[John Ferlan]

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)