On Wed, Jul 12, 2023 at 05:02:03PM +0100, Anthony PERARD wrote:
> 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.
I wasn't planning on changing the code, as this is just indentation
movement, but will do.
> 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 <[email protected]>
Thanks, Roger.
