[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 09/11] hypervisor: Remove redundant validity checks



John Ferlan wrote:
> Arguments for driver entry points are checked in libvirt.c, so no need to
> check again.
> ---
>  src/xen/xen_hypervisor.c | 164 +++++++++--------------------------------------
>  1 file changed, 30 insertions(+), 134 deletions(-)
>
> diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
> index bfee56d..2437413 100644
> --- a/src/xen/xen_hypervisor.c
> +++ b/src/xen/xen_hypervisor.c
> @@ -1161,15 +1161,8 @@ char *
>  xenHypervisorGetSchedulerType(virDomainPtr domain, int *nparams)
>  {
>      char *schedulertype = NULL;
> -    xenUnifiedPrivatePtr priv;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
>  
> -    if (domain->conn == NULL) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("domain or conn is NULL"));
> -        return NULL;
> -    }
> -
> -    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
>      if (priv->handle < 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                         _("priv->handle invalid"));
> @@ -1242,15 +1235,8 @@ int
>  xenHypervisorGetSchedulerParameters(virDomainPtr domain,
>                                      virTypedParameterPtr params, int *nparams)
>  {
> -    xenUnifiedPrivatePtr priv;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
>  
> -    if (domain->conn == NULL) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("domain or conn is NULL"));
> -        return -1;
> -    }
> -
> -    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
>      if (priv->handle < 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                         _("priv->handle invalid"));
> @@ -1350,12 +1336,6 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain,
>      xenUnifiedPrivatePtr priv;
>      char buf[256];
>  
> -    if (domain->conn == NULL) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("domain or conn is NULL"));
> -        return -1;
> -    }
> -
>      if (nparams == 0) {
>          /* nothing to do, exit early */
>          return 0;
> @@ -2252,12 +2232,7 @@ int
>  xenHypervisorClose(virConnectPtr conn)
>  {
>      int ret;
> -    xenUnifiedPrivatePtr priv;
> -
> -    if (conn == NULL)
> -        return -1;
> -
> -    priv = (xenUnifiedPrivatePtr) conn->privateData;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
>   

Extra space here in the cast.

>  
>      if (priv->handle < 0)
>          return -1;
> @@ -2282,12 +2257,9 @@ xenHypervisorClose(virConnectPtr conn)
>  int
>  xenHypervisorGetVersion(virConnectPtr conn, unsigned long *hvVer)
>  {
> -    xenUnifiedPrivatePtr priv;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
>   

Same here, along with some other below. ACK with those nits fixed.

Regards,
Jim

>  
> -    if (conn == NULL)
> -        return -1;
> -    priv = (xenUnifiedPrivatePtr) conn->privateData;
> -    if (priv->handle < 0 || hvVer == NULL)
> +    if (priv->handle < 0)
>          return -1;
>      *hvVer = (hv_versions.hv >> 16) * 1000000 + (hv_versions.hv & 0xFFFF) * 1000;
>      return 0;
> @@ -2803,11 +2775,8 @@ xenHypervisorNumOfDomains(virConnectPtr conn)
>      int ret, nbids;
>      static int last_maxids = 2;
>      int maxids = last_maxids;
> -    xenUnifiedPrivatePtr priv;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
>  
> -    if (conn == NULL)
> -        return -1;
> -    priv = (xenUnifiedPrivatePtr) conn->privateData;
>      if (priv->handle < 0)
>          return -1;
>  
> @@ -2860,14 +2829,9 @@ xenHypervisorListDomains(virConnectPtr conn, int *ids, int maxids)
>  {
>      xen_getdomaininfolist dominfos;
>      int ret, nbids, i;
> -    xenUnifiedPrivatePtr priv;
> -
> -    if (conn == NULL)
> -        return -1;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
>  
> -    priv = (xenUnifiedPrivatePtr) conn->privateData;
> -    if (priv->handle < 0 ||
> -        (ids == NULL) || (maxids < 0))
> +    if (priv->handle < 0)
>          return -1;
>  
>      if (maxids == 0)
> @@ -3084,11 +3048,8 @@ int
>  xenHypervisorGetMaxVcpus(virConnectPtr conn,
>                           const char *type ATTRIBUTE_UNUSED)
>  {
> -    xenUnifiedPrivatePtr priv;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
>  
> -    if (conn == NULL)
> -        return -1;
> -    priv = (xenUnifiedPrivatePtr) conn->privateData;
>      if (priv->handle < 0)
>          return -1;
>  
> @@ -3108,14 +3069,10 @@ xenHypervisorGetMaxVcpus(virConnectPtr conn,
>  unsigned long
>  xenHypervisorGetDomMaxMemory(virConnectPtr conn, int id)
>  {
> -    xenUnifiedPrivatePtr priv;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
>      xen_getdomaininfo dominfo;
>      int ret;
>  
> -    if (conn == NULL)
> -        return 0;
> -
> -    priv = (xenUnifiedPrivatePtr) conn->privateData;
>      if (priv->handle < 0)
>          return 0;
>  
> @@ -3148,12 +3105,8 @@ xenHypervisorGetDomMaxMemory(virConnectPtr conn, int id)
>  static unsigned long long ATTRIBUTE_NONNULL(1)
>  xenHypervisorGetMaxMemory(virDomainPtr domain)
>  {
> -    xenUnifiedPrivatePtr priv;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
>  
> -    if (domain->conn == NULL)
> -        return 0;
> -
> -    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
>      if (priv->handle < 0 || domain->id < 0)
>          return 0;
>  
> @@ -3173,7 +3126,7 @@ xenHypervisorGetMaxMemory(virDomainPtr domain)
>  int
>  xenHypervisorGetDomInfo(virConnectPtr conn, int id, virDomainInfoPtr info)
>  {
> -    xenUnifiedPrivatePtr priv;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
>      xen_getdomaininfo dominfo;
>      int ret;
>      uint32_t domain_flags, domain_state, domain_shutdown_cause;
> @@ -3184,11 +3137,7 @@ xenHypervisorGetDomInfo(virConnectPtr conn, int id, virDomainInfoPtr info)
>              kb_per_pages = 4;
>      }
>  
> -    if (conn == NULL)
> -        return -1;
> -
> -    priv = (xenUnifiedPrivatePtr) conn->privateData;
> -    if (priv->handle < 0 || info == NULL)
> +    if (priv->handle < 0)
>          return -1;
>  
>      memset(info, 0, sizeof(virDomainInfo));
> @@ -3256,14 +3205,9 @@ xenHypervisorGetDomInfo(virConnectPtr conn, int id, virDomainInfoPtr info)
>  int
>  xenHypervisorGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info)
>  {
> -    xenUnifiedPrivatePtr priv;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
>  
> -    if (domain->conn == NULL)
> -        return -1;
> -
> -    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
> -    if (priv->handle < 0 || info == NULL ||
> -        (domain->id < 0))
> +    if (priv->handle < 0 || domain->id < 0)
>          return -1;
>  
>      return xenHypervisorGetDomInfo(domain->conn, domain->id, info);
> @@ -3287,14 +3231,11 @@ xenHypervisorGetDomainState(virDomainPtr domain,
>                              int *reason,
>                              unsigned int flags)
>  {
> -    xenUnifiedPrivatePtr priv = domain->conn->privateData;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
>      virDomainInfo info;
>  
>      virCheckFlags(0, -1);
>  
> -    if (domain->conn == NULL)
> -        return -1;
> -
>      if (priv->handle < 0 || domain->id < 0)
>          return -1;
>  
> @@ -3326,19 +3267,13 @@ xenHypervisorGetDomainState(virDomainPtr domain,
>   * Returns the number of entries filled in freeMems, or -1 in case of error.
>   */
>  int
> -xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *freeMems,
> +xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn,
> +                                    unsigned long long *freeMems,
>                                      int startCell, int maxCells)
>  {
>      xen_op_v2_sys op_sys;
>      int i, j, ret;
> -    xenUnifiedPrivatePtr priv;
> -
> -    if (conn == NULL) {
> -        virReportError(VIR_ERR_INVALID_ARG, "%s", _("invalid argument"));
> -        return -1;
> -    }
> -
> -    priv = conn->privateData;
> +    xenUnifiedPrivatePtr priv = conn->privateData;
>  
>      if (priv->nbNodeCells < 0) {
>          virReportError(VIR_ERR_XEN_CALL, "%s",
> @@ -3400,12 +3335,8 @@ int
>  xenHypervisorPauseDomain(virDomainPtr domain)
>  {
>      int ret;
> -    xenUnifiedPrivatePtr priv;
> -
> -    if (domain->conn == NULL)
> -        return -1;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
>  
> -    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
>      if (priv->handle < 0 || domain->id < 0)
>          return -1;
>  
> @@ -3427,12 +3358,8 @@ int
>  xenHypervisorResumeDomain(virDomainPtr domain)
>  {
>      int ret;
> -    xenUnifiedPrivatePtr priv;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
>  
> -    if (domain->conn == NULL)
> -        return -1;
> -
> -    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
>      if (priv->handle < 0 || domain->id < 0)
>          return -1;
>  
> @@ -3459,14 +3386,10 @@ xenHypervisorDestroyDomainFlags(virDomainPtr domain,
>                                  unsigned int flags)
>  {
>      int ret;
> -    xenUnifiedPrivatePtr priv;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
>  
>      virCheckFlags(0, -1);
>  
> -    if (domain->conn == NULL)
> -        return -1;
> -
> -    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
>      if (priv->handle < 0 || domain->id < 0)
>          return -1;
>  
> @@ -3489,12 +3412,8 @@ int
>  xenHypervisorSetMaxMemory(virDomainPtr domain, unsigned long memory)
>  {
>      int ret;
> -    xenUnifiedPrivatePtr priv;
> -
> -    if (domain->conn == NULL)
> -        return -1;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
>  
> -    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
>      if (priv->handle < 0 || domain->id < 0)
>          return -1;
>  
> @@ -3519,12 +3438,8 @@ int
>  xenHypervisorSetVcpus(virDomainPtr domain, unsigned int nvcpus)
>  {
>      int ret;
> -    xenUnifiedPrivatePtr priv;
> -
> -    if (domain->conn == NULL)
> -        return -1;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
>  
> -    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
>      if (priv->handle < 0 || domain->id < 0 || nvcpus < 1)
>          return -1;
>  
> @@ -3551,14 +3466,9 @@ xenHypervisorPinVcpu(virDomainPtr domain, unsigned int vcpu,
>                       unsigned char *cpumap, int maplen)
>  {
>      int ret;
> -    xenUnifiedPrivatePtr priv;
> -
> -    if (domain->conn == NULL)
> -        return -1;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
>  
> -    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
> -    if (priv->handle < 0 || (domain->id < 0) ||
> -        (cpumap == NULL) || (maplen < 1))
> +    if (priv->handle < 0 || domain->id < 0)
>          return -1;
>  
>      ret = virXen_setvcpumap(priv->handle, domain->id, vcpu,
> @@ -3593,26 +3503,16 @@ xenHypervisorGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo,
>  {
>      xen_getdomaininfo dominfo;
>      int ret;
> -    xenUnifiedPrivatePtr priv;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
>      virVcpuInfoPtr ipt;
>      int nbinfo, i;
>  
> -    if (domain->conn == NULL)
> -        return -1;
> -
> -    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
> -    if (priv->handle < 0 || (domain->id < 0) ||
> -        (info == NULL) || (maxinfo < 1) ||
> -        (sizeof(cpumap_t) & 7)) {
> +    if (priv->handle < 0 || domain->id < 0 || sizeof(cpumap_t) & 7) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                         _("domain shut off or invalid"));
>          return -1;
>      }
> -    if ((cpumaps != NULL) && (maplen < 1)) {
> -        virReportError(VIR_ERR_INVALID_ARG, "%s",
> -                       _("invalid argument"));
> -        return -1;
> -    }
> +
>      /* first get the number of virtual CPUs in this domain */
>      XEN_GETDOMAININFO_CLEAR(dominfo);
>      ret = virXen_getdomaininfo(priv->handle, domain->id,
> @@ -3667,12 +3567,8 @@ xenHypervisorGetVcpuMax(virDomainPtr domain)
>      xen_getdomaininfo dominfo;
>      int ret;
>      int maxcpu;
> -    xenUnifiedPrivatePtr priv;
> +    xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
>  
> -    if (domain->conn == NULL)
> -        return -1;
> -
> -    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
>      if (priv->handle < 0)
>          return -1;
>  
>   


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]