[libvirt] [PATCH] simplify block of codes

[Alex Jia]

On 01/30/2012 10:41 AM, Laine Stump wrote:
> 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.
Right, I'm missing VIR_FREE(params);
>
> 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".
Yeah, you're right, 'i' should be more exact, thanks for your comment.

Thanks,
Alex
>
>
> 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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list