[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 08/05/13 21:38, John Ferlan wrote:
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

I'd think this is personal habit, and actually we use it across the
source.


      }
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;

Oops...


@@ -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;

Okay.


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

--
libvir-list mailing list
libvir-list redhat com
https://www.redhat.com/mailman/listinfo/libvir-list


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