Re: [libvirt] PATCH: 4/25: Cleanup exit paths in remote driver

[Jim Meyering <jim meyering net>]

"Daniel P. Berrange" <berrange redhat com> wrote:
> This patch ensures all public API methods only have a single exit
> path, to make mutex unlocking simpler.
>
>  remote_internal.c | 1256 +++++++++++++++++++++++++++++++++++-------------------

Hi Dan,

I got about halfway through it and spotted something odd:

> diff --git a/src/remote_internal.c b/src/remote_internal.c
...
>  static int
>  remoteDomainGetSchedulerParameters (virDomainPtr domain,
>                                      virSchedParameterPtr params, int *nparams)
...
> +cleanup:
>      xdr_free ((xdrproc_t) xdr_remote_domain_get_scheduler_parameters_ret, (char *) &ret);
> -    return 0;
> +    if (rv != 0) {
> +        for ( ; i >= 0 ; i--)
> +            VIR_FREE(params[i].field);
> +    }
> +
> +done:
> +    return rv;
>  }

Freeing the .field member looks bogus.
That member is declared like this:

  struct _virSchedParameter {
      char field[VIR_DOMAIN_SCHED_FIELD_LENGTH];  /* parameter name */

merge problem, or vestige of older design, I guess.