Re: [libvirt] [PATCH] simplify block of codes

On 01/29/2012 04:35 AM, ajia(a)redhat.com wrote: > From: Alex Jia&lt;ajia(a)redhat.com&gt; > > Using new function 'virTypedParameterArrayClear' to simplify block of > codes. > > * daemon/remote.c, src/remote/remote_driver.c: simplify codes. > > Signed-off-by: Alex Jia&lt;ajia(a)redhat.com&gt; > --- > 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); -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list