Re: [PATCH v2 2/6] libxl: change the type of libxl_cpuid_policy_list

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Tue, Jul 11, 2023 at 11:22:26AM +0200, Roger Pau Monne wrote:
> -void libxl_cpuid_dispose(libxl_cpuid_policy_list *p_cpuid_list)
> +void libxl_cpuid_dispose(libxl_cpuid_policy_list *pl)
>  {
> -    int i, j;
> -    libxl_cpuid_policy_list cpuid_list = *p_cpuid_list;
> +    libxl_cpuid_policy_list policy = *pl;
>  
> -    if (cpuid_list == NULL)
> +    if (policy == NULL)
>          return;
> -    for (i = 0; cpuid_list[i].input[0] != XEN_CPUID_INPUT_UNUSED; i++) {
> -        for (j = 0; j < 4; j++)
> -            if (cpuid_list[i].policy[j] != NULL) {
> -                free(cpuid_list[i].policy[j]);
> -                cpuid_list[i].policy[j] = NULL;
> -            }
> +
> +    if (policy->cpuid) {
> +        unsigned int i, j;
> +        struct xc_xend_cpuid *cpuid_list = policy->cpuid;
> +
> +        for (i = 0; cpuid_list[i].input[0] != XEN_CPUID_INPUT_UNUSED; i++) {
> +            for (j = 0; j < 4; j++)
> +                if (cpuid_list[i].policy[j] != NULL) {
> +                    free(cpuid_list[i].policy[j]);
> +                    cpuid_list[i].policy[j] = NULL;
> +                }

This looks like a lot of work. We could just call
free(cpuid_list[i].policy[j]) and that's all, as cpuid_list will be gone
right after the loop.

Also, please add {} for the second "for ()" loop.

> +        }
> +        free(policy->cpuid);
>      }

Beside some the coding style pointing out, the patch looks fine:
Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

Thanks,

-- 
Anthony PERARD