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

Re: [libvirt] [PATCH 4/7] util: Change virIsCapable* to return bool



On 05/06/2013 08:45 AM, Osier Yang wrote:
> Function name with "aIsB" generally means its return value is
> in Bi-state (true/false).
> ---
>  src/node_device/node_device_linux_sysfs.c |  4 ++--
>  src/util/virutil.c                        | 18 +++++++++---------
>  src/util/virutil.h                        |  4 ++--
>  3 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c
> index 5228a01..cb2f86e 100644
> --- a/src/node_device/node_device_linux_sysfs.c
> +++ b/src/node_device/node_device_linux_sysfs.c
> @@ -47,7 +47,7 @@ detect_scsi_host_caps(union _virNodeDevCapData *d)
>  
>      VIR_DEBUG("Checking if host%d is an FC HBA", d->scsi_host.host);
>  
> -    if (virIsCapableFCHost(NULL, d->scsi_host.host) == 0) {
> +    if (virIsCapableFCHost(NULL, d->scsi_host.host)) {
>          d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST;
>  
>          if (virReadFCHost(NULL,
> @@ -76,7 +76,7 @@ detect_scsi_host_caps(union _virNodeDevCapData *d)
>          }
>      }
>  
> -    if (virIsCapableVport(NULL, d->scsi_host.host) == 0) {
> +    if (virIsCapableVport(NULL, d->scsi_host.host)) {
>          d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS;
>  
>          if (virReadFCHost(NULL,
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index da87897..dfbef06 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -3133,12 +3133,12 @@ cleanup:
>      return ret;
>  }
>  
> -int
> +bool
>  virIsCapableFCHost(const char *sysfs_prefix,
>                     int host)
>  {
>      char *sysfs_path = NULL;
> -    int ret = -1;
> +    bool ret = false;
>  
>      if (virAsprintf(&sysfs_path, "%s/host%d",
>                      sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH,

Doesn't this return -1 here? after the virReportOOMError()

So you'd need a return ret; instead


> @@ -3148,19 +3148,19 @@ virIsCapableFCHost(const char *sysfs_prefix,
>      }
>  
>      if (access(sysfs_path, F_OK) == 0)
> -        ret = 0;
> +        ret = true;
>  
>      VIR_FREE(sysfs_path);
>      return ret;
>  }
>  
> -int
> +bool
>  virIsCapableVport(const char *sysfs_prefix,
>                    int host)
>  {
>      char *scsi_host_path = NULL;
>      char *fc_host_path = NULL;
> -    int ret = -1;
> +    int ret = false;
>  
>      if (virAsprintf(&fc_host_path,
>                      "%s/host%d/%s",
> @@ -3168,7 +3168,7 @@ virIsCapableVport(const char *sysfs_prefix,
>                      host,
>                      "vport_create") < 0) {
>          virReportOOMError();
> -        return -1;
> +        return false;

s/false/ret

or

goto cleanup; instead

>      }
>  
>      if (virAsprintf(&scsi_host_path,
> @@ -3182,7 +3182,7 @@ virIsCapableVport(const char *sysfs_prefix,
>  
>      if ((access(fc_host_path, F_OK) == 0) ||
>          (access(scsi_host_path, F_OK) == 0))
> -        ret = 0;
> +        ret = true;
>  
>  cleanup:
>      VIR_FREE(fc_host_path);
> @@ -3458,7 +3458,7 @@ virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED,
>      return -1;
>  }
>  
> -int
> +bool
>  virIsCapableFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED,
>                     int host ATTRIBUTE_UNUSED)
>  {

You forgot the:

s/return -1;/return false;

> @@ -3466,7 +3466,7 @@ virIsCapableFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED,
>      return -1;
>  }
>  
> -int
> +bool
>  virIsCapableVport(const char *sysfs_prefix ATTRIBUTE_UNUSED,
>                    int host ATTRIBUTE_UNUSED)
>  {

You forgot the:

s/return -1;/return false;

> diff --git a/src/util/virutil.h b/src/util/virutil.h
> index 8a2d25c..0e45ac7 100644
> --- a/src/util/virutil.h
> +++ b/src/util/virutil.h
> @@ -254,8 +254,8 @@ int virReadFCHost(const char *sysfs_prefix,
>                    char **result)
>      ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
>  
> -int virIsCapableFCHost(const char *sysfs_prefix, int host);
> -int virIsCapableVport(const char *sysfs_prefix, int host);
> +bool virIsCapableFCHost(const char *sysfs_prefix, int host);
> +bool virIsCapableVport(const char *sysfs_prefix, int host);
>  
>  enum {
>      VPORT_CREATE,
> 

ACK w/ changes

John


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