[libvirt] [PATCH 2/3] vmx: The virVMXParseDisk deviceType can be NULL, add some missing checks

[Michal Privoznik]

On 05.09.2015 16:29, Matthias Bolte wrote:
> 2015-09-02 13:00 GMT+02:00 Michal Privoznik <mprivozn at redhat.com>:
>> On 01.09.2015 16:52, Matthias Bolte wrote:
>>> ---
>>>  src/vmx/vmx.c | 22 +++++++++++-----------
>>>  1 file changed, 11 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
>>> index ba4d046..9d1574f 100644
>>> --- a/src/vmx/vmx.c
>>> +++ b/src/vmx/vmx.c
>>> @@ -2178,8 +2178,9 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
>>>                  (*def)->transient = STRCASEEQ(mode,
>>>                                                "independent-nonpersistent");
>>>          } else if (virFileHasSuffix(fileName, ".iso") ||
>>> -                   STRCASEEQ(deviceType, "atapi-cdrom") ||
>>> -                   STRCASEEQ(deviceType, "cdrom-raw")) {
>>> +                   (deviceType &&
>>> +                    (STRCASEEQ(deviceType, "atapi-cdrom") ||
>>> +                     STRCASEEQ(deviceType, "cdrom-raw")))) {
>>>              /*
>>>               * This function was called in order to parse a harddisk device,
>>>               * but .iso files, 'atapi-cdrom', and 'cdrom-raw' devices are for
>>> @@ -2199,13 +2200,12 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
>>>          if (virFileHasSuffix(fileName, ".iso")) {
>>>              char *tmp;
>>>
>>> -            if (deviceType != NULL) {
>>> -                if (STRCASENEQ(deviceType, "cdrom-image")) {
>>> -                    virReportError(VIR_ERR_INTERNAL_ERROR,
>>> -                                   _("Expecting VMX entry '%s' to be 'cdrom-image' "
>>> -                                     "but found '%s'"), deviceType_name, deviceType);
>>> -                    goto cleanup;
>>> -                }
>>> +            if (deviceType && STRCASENEQ(deviceType, "cdrom-image")) {
>>> +                virReportError(VIR_ERR_INTERNAL_ERROR,
>>> +                               _("Expecting VMX entry '%s' to be 'cdrom-image' "
>>> +                                 "but found '%s'"), deviceType_name,
>>> +                               deviceType ? deviceType : "unknown");
>>
>> The control can get here iff deviceType != NULL.
>>
> 
> The check for NULL was preexisting here, I just merged two nested if
> blocks here. Also I don't see how you get to the conclusion that the
> control can only get here if deviceType != NULL. There is no code
> before or around this block that would do that.
> 

Maybe I should have been more specific. I meant the virReportError().
There's no need for (preexisting) ternary operator since the if() right
above checks for deviceType.

Michal