[libvirt] [PATCH] simplify block of codes

[Laine Stump]

On 01/29/2012 04:35 AM, ajia at redhat.com wrote:
> From: Alex Jia<ajia at redhat.com>
>
> Using new function 'virTypedParameterArrayClear' to simplify block of codes.
>
> * daemon/remote.c, src/remote/remote_driver.c: simplify codes.
>
> Signed-off-by: Alex Jia<ajia at redhat.com>
> ---
>   daemon/remote.c            |   10 ++--------
>   src/remote/remote_driver.c |   16 ++++------------
>   2 files changed, 6 insertions(+), 20 deletions(-)
>
> diff --git a/daemon/remote.c b/daemon/remote.c
> index cb8423a..7e90bd7 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -847,14 +847,8 @@ remoteDeserializeTypedParameters(remote_typed_param *args_params_val,
>       rv = 0;
>
>   cleanup:
> -    if (rv<  0) {
> -        int j;
> -        for (j = 0; j<  i; ++j) {
> -            if (params[j].type == VIR_TYPED_PARAM_STRING)
> -                VIR_FREE(params[j].value.s);
> -        }
> -        VIR_FREE(params);
> -    }
> +    if (rv<  0)
> +        virTypedParameterArrayClear(params, *nparams);
>       return params;

This is wrong in two ways:

1) The modification ends up not freeing params itself, only clearing the 
items it points to.

2) If there's an error when i < nparams, This new code will try to clear 
out too many items - the proper number to clear is "i", not "*nparams".

What you really want is:

     if (rv < 0) {
         virTypedParameterArrayClear(params, i);
         VIR_FREE(params);
     }


>   }
>
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index 61b96e9..15a20ff 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -46,6 +46,7 @@
>   #include "virfile.h"
>   #include "command.h"
>   #include "intprops.h"
> +#include "virtypedparam.h"
>
>   #define VIR_FROM_THIS VIR_FROM_REMOTE
>
> @@ -1417,12 +1418,8 @@ remoteDeserializeTypedParameters(remote_typed_param *ret_params_val,
>       rv = 0;
>
>   cleanup:
> -    if (rv<  0) {
> -        int j;
> -        for (j = 0; j<  i; j++)
> -            if (params[j].type == VIR_TYPED_PARAM_STRING)
> -                VIR_FREE(params[j].value.s);
> -    }
> +    if (rv<  0)
> +        virTypedParameterArrayClear(params, *nparams);
>       return rv;
>   }
>
> @@ -2386,12 +2383,7 @@ static int remoteDomainGetCPUStats(virDomainPtr domain,
>   cleanup:
>       if (rv<  0) {
>           int max = nparams * ncpus;
> -        int i;
> -
> -        for (i = 0; i<  max; i++) {
> -            if (params[i].type == VIR_TYPED_PARAM_STRING)
> -                VIR_FREE(params[i].value.s);
> -        }
> +        virTypedParameterArrayClear(params, max);
>       }
>       xdr_free ((xdrproc_t) xdr_remote_domain_get_cpu_stats_ret,
>                 (char *)&ret);