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

2015-09-02 13:00 GMT+02:00 Michal Privoznik <mprivozn(a)redhat.com&gt;: > 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.