[libvirt] Re: [PATCH] Remove unsafe strncpy from esx_vmx.c

[Matthias Bolte]

2009/8/7 Chris Lalancette <clalance at redhat.com>:
> While trying to remove uses of unsafe strncpy in the tree, I came
> across a couple of usage in the ESX driver.  To my eyes, the snprintf
> replacements do the same thing in less code, and are also safer.
> This has been compile tested only.  Mattias, is there any reason we
> can't use these simpler (and safer) string routines?
>
> Signed-off-by: Chris Lalancette <clalance at redhat.com>
> Cc: Mattias Bolte <matthias.bolte at googlemail.com>
> ---
>  src/esx/esx_vmx.c |   22 +++++++---------------
>  1 files changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c
> index af5234e..91a86e2 100644
> --- a/src/esx/esx_vmx.c
> +++ b/src/esx/esx_vmx.c
> @@ -397,10 +397,7 @@ def->parallels[0]...
>
>  #define ESX_BUILD_VMX_NAME(_suffix)                                           \
>     do {                                                                      \
> -        strncpy(_suffix##_name, prefix, sizeof (_suffix##_name) - 1);         \
> -        _suffix##_name[sizeof (_suffix##_name) - 1] = '\0';                   \
> -        strncat(_suffix##_name, "."#_suffix,                                  \
> -                sizeof (_suffix##_name) - 1 - strlen(_suffix##_name));        \
> +        snprintf(_suffix##_name, sizeof(_suffix##_name), "%s."#_suffix, prefix); \
>     } while (0)

The do/while can be removed as the macro expands to only one statement now:

#define ESX_BUILD_VMX_NAME(_suffix)                                           \
    snprintf(_suffix##_name, sizeof(_suffix##_name), "%s."#_suffix, prefix)   \

> @@ -839,11 +836,9 @@ esxVMX_ParseSCSIController(virConnectPtr conn, virConfPtr conf, int controller,
>         goto failure;
>     }
>
> -    strncpy(present_name, "scsiX.present", sizeof (virtualDev_name));
> -    strncpy(virtualDev_name, "scsiX.virtualDev", sizeof (virtualDev_name));
> -
> -    present_name[4] = '0' + controller;
> -    virtualDev_name[4] = '0' + controller;
> +    snprintf(present_name, sizeof(present_name), "scsi%d.present", controller);
> +    snprintf(virtualDev_name, sizeof(virtualDev_name), "scsi%d.virtualDev",
> +             controller);
>
>     if (esxUtil_GetConfigBoolean(conn, conf, present_name, present, 0, 1) < 0) {
>         goto failure;
> @@ -1333,8 +1328,7 @@ esxVMX_ParseEthernet(virConnectPtr conn, virConfPtr conf, int controller,
>         goto failure;
>     }
>
> -    strncpy(prefix, "ethernetX", sizeof (prefix));
> -    prefix[8] = '0' + controller;
> +    snprintf(prefix, sizeof(prefix), "ethernet%d", controller);
>
>     ESX_BUILD_VMX_NAME(present);
>     ESX_BUILD_VMX_NAME(startConnected);
> @@ -1514,8 +1508,7 @@ esxVMX_ParseSerial(virConnectPtr conn, virConfPtr conf, int port,
>         goto failure;
>     }
>
> -    strncpy(prefix, "serialX", sizeof (prefix));
> -    prefix[6] = '0' + port;
> +    snprintf(prefix, sizeof(prefix), "serial%d", port);
>
>     ESX_BUILD_VMX_NAME(present);
>     ESX_BUILD_VMX_NAME(startConnected);
> @@ -1627,8 +1620,7 @@ esxVMX_ParseParallel(virConnectPtr conn, virConfPtr conf, int port,
>         goto failure;
>     }
>
> -    strncpy(prefix, "parallelX", sizeof (prefix));
> -    prefix[8] = '0' + port;
> +    snprintf(prefix, sizeof(prefix), "parallel%d", port);
>
>     ESX_BUILD_VMX_NAME(present);
>     ESX_BUILD_VMX_NAME(startConnected);
> --
> 1.6.0.6
>

Just tested it, no objections beside the cosmetic one (do/while), ACK.

Matthias