Re: [PATCH 01/26] qemu_driver: Resolve Coverity COPY_PASTE_ERROR

[Peter Krempa <pkrempa@xxxxxxxxxx>]

On 09/05/14 00:26, John Ferlan wrote:
> In qemuDomainSetBlkioParameters(), Coverity points out that the calls
> to qemuDomainParseBlkioDeviceStr() are slightly different and points
> out there may be a cut-n-paste error.
> 
> In the first call (AFFECT_LIVE), the second parameter is "param->field";
> however, for the second call (AFFECT_CONFIG), the second parameter is
> "params->field".  It seems the "param->field" is correct especially since
> each path as a setting of "param" to "&params[i]".  Furthermore, there
> were a few more instances of using "params[i]" instead of "param->"
> which I cleaned up.
> 
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
>  src/qemu/qemu_driver.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 5121f85..f52f00d 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7825,7 +7825,7 @@ qemuDomainSetBlkioParameters(virDomainPtr dom,
>              virTypedParameterPtr param = &params[i];
>  
>              if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) {
> -                if (virCgroupSetBlkioWeight(priv->cgroup, params[i].value.ui) < 0)
> +                if (virCgroupSetBlkioWeight(priv->cgroup, param->value.ui) < 0)
>                      ret = -1;
>              } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) ||
>                         STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) ||
> @@ -7836,7 +7836,7 @@ qemuDomainSetBlkioParameters(virDomainPtr dom,
>                  virBlkioDevicePtr devices = NULL;
>                  size_t j;
>  
> -                if (qemuDomainParseBlkioDeviceStr(params[i].value.s,
> +                if (qemuDomainParseBlkioDeviceStr(param->value.s,
>                                                    param->field,
>                                                    &devices,
>                                                    &ndevices) < 0) {
> @@ -7919,7 +7919,7 @@ qemuDomainSetBlkioParameters(virDomainPtr dom,
>              virTypedParameterPtr param = &params[i];
>  
>              if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) {
> -                persistentDef->blkio.weight = params[i].value.ui;
> +                persistentDef->blkio.weight = param->value.ui;
>              } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) ||
>                         STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) ||
>                         STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) ||
> @@ -7928,8 +7928,8 @@ qemuDomainSetBlkioParameters(virDomainPtr dom,
>                  virBlkioDevicePtr devices = NULL;
>                  size_t ndevices;
>  
> -                if (qemuDomainParseBlkioDeviceStr(params[i].value.s,
> -                                                  params->field,
> +                if (qemuDomainParseBlkioDeviceStr(param->value.s,
> +                                                  param->field,

wow, this was the real bug here. The original code would use the "field"
value of the first element in the params array. Very easy to overlook.

>                                                    &devices,
>                                                    &ndevices) < 0) {
>                      ret = -1;
> 

ACK.

Peter

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list