[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 5/5] esxVMX_GatherSCSIControllers: avoid NULL dereference



Matthias Bolte wrote:
> 2010/4/14 Jim Meyering <jim meyering net>:
>> From: Jim Meyering <meyering redhat com>
>>
>> * src/esx/esx_vmx.c (esxVMX_GatherSCSIControllers): Do not dereference
>> a NULL disk->driverName.  We already detect this condition in another
>> case.  Check for it here, too.
>> ---
>>  src/esx/esx_vmx.c |    5 +++--
>>  1 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c
>> index 647e720..aed5cc1 100644
>> --- a/src/esx/esx_vmx.c
>> +++ b/src/esx/esx_vmx.c
>> @@ -570,11 +570,12 @@ esxVMX_GatherSCSIControllers(virDomainDefPtr def, char *virtualDev[4],
>>
>>         if (virtualDev[controller] == NULL) {
>>             virtualDev[controller] = disk->driverName;
>> -        } else if (STRCASENEQ(virtualDev[controller], disk->driverName)) {
>> +        } else if (disk->driverName == NULL
>> +                   || STRCASENEQ(virtualDev[controller], disk->driverName)) {
>
> Style nitpick: I'd like to have the || at the end of the previous
> line, like in the rest of this file.

Ok.  Adjusted.
However, note that there is a good (human-factors-related)
reason to put operators at the beginning of a line rather
than at the end like that: it makes them more apparent.
Sure, some of the time you sacrifice a little on the alignment front,
but that is outweighed by the improved readability.

It's similar to the 80-column max rule: things at the end of a line
(especially when wrapped) are far easier to miss.

>>             ESX_ERROR(VIR_ERR_INTERNAL_ERROR,
>>                       _("Inconsistent driver usage ('%s' is not '%s') on SCSI "
>>                         "controller index %d"), virtualDev[controller],
>> -                      disk->driverName, controller);
>> +                      disk->driverName ? disk->driverName : "?", controller);
>>             return -1;
>>         }
>>     }
>> --
>> 1.7.1.rc1.248.gcefbb
>
> ACK.

Thanks. Pushed.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]