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

[Osier Yang]

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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list