Re: [libvirt] [PATCH 2/6] Clarify the semantic of virDomainGetSchedulerParameters arguments

On Wed, May 18, 2011 at 12:21:09PM +0200, Matthias Bolte wrote: > params and nparams are essential and cannot be NULL. Check this in > libvirt.c and remove redundant checks from the drivers (e.g. xend). > > Instead of enforcing that nparams must point to exact same value as > returned by virDomainGetSchedulerType relax this to a lower bound > check. This is what some drivers (e.g. xen hypervisor and esx) > already did. Other drivers (e.g. xend) didn't check nparams at all > and assumed that there is enough space in params. > > Unify the behavior in all drivers to a lower bound check and update > nparams to the number of valid values in params on success. [...] > - * Get the scheduler parameters, the @params array will be filled with the > - * values. > + * Get all scheduler parameters, the @params array will be filled with the > + * values and @nparams will be updated to the number of valid elements in > + * @params. >   * >   * Returns -1 in case of error, 0 in case of success. >   */ > @@ -5041,6 +5042,12 @@ virDomainGetSchedulerParameters(virDomainPtr domain, >          virDispatchError(NULL); >          return -1; >      } > + > +    if (params == NULL || nparams == NULL || *nparams < 0) {  Hum, what would *nparams == 0 mean ? The arry pointer has to be allocated anyway. Maybe we should use *nparams <= 0 here

> +        virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); > +        goto error; > +    } > + >      conn = domain->conn; > >      if (conn->driver->domainGetSchedulerParameters) { > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index 6b2d806..b85c743 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -2466,7 +2466,7 @@ libxlDomainGetSchedulerParameters(virDomainPtr dom, virSchedParameterPtr params, >          goto cleanup; >      } > > -    if (*nparams != XEN_SCHED_CREDIT_NPARAM) { > +    if (*nparams < XEN_SCHED_CREDIT_NPARAM) { >          libxlError(VIR_ERR_INVALID_ARG, "%s", _("Invalid parameter count")); >          goto cleanup; >      } > @@ -2494,6 +2494,7 @@ libxlDomainGetSchedulerParameters(virDomainPtr dom, virSchedParameterPtr params, >          goto cleanup; >      } > > +    *nparams = XEN_SCHED_CREDIT_NPARAM; >      ret = 0; > >  cleanup: > diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c > index 2bb592d..a65299b 100644 > --- a/src/lxc/lxc_driver.c > +++ b/src/lxc/lxc_driver.c > @@ -2234,7 +2234,7 @@ static int lxcGetSchedulerParameters(virDomainPtr domain, >      if (driver->cgroup == NULL) >          return -1; > > -    if ((*nparams) != 1) { > +    if (*nparams < 1) {  which is what we are doing here and in all the drivers. ACK, with that nit Daniel