[libvirt] [PATCH 2/3] vmx: The virVMXParseDisk deviceType can be NULL, add some missing checks
Matthias Bolte
matthias.bolte at googlemail.com
Mon Sep 7 20:20:54 UTC 2015
2015-09-07 8:28 GMT+02:00 Michal Privoznik <mprivozn at redhat.com>:
> 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.
Ah, sorry I totally missed that one. I removed it now.
--
Matthias Bolte
http://photron.blogspot.com
More information about the libvir-list
mailing list