9:56 a.m.
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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list