Re: [libvirt] [PATCH 5/5] esxVMX_GatherSCSIControllers: avoid NULL dereference
Matthias Bolte wrote:
2010/4/14 Jim Meyering <jim(a)meyering.net>:
> From: Jim Meyering <meyering(a)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.