2:48 a.m.
virDrvSupportsFeature API is allowed to return -1 on error while all but
one uses of VIR_DRV_SUPPORTS_FEATURE only check for (non)zero return
value. Let's make this macro return zero on error, which is what
everyone expects anyway.
---
src/driver.h | 8 ++++----
src/libvirt.c | 5 ++++-
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/driver.h b/src/driver.h
index b770e5e..e797a75 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -52,12 +52,12 @@ typedef enum {
* Note that you must check for errors.
*
* Returns:
- * >= 1 Feature is supported.
+ * != 0 Feature is supported.
* 0 Feature is not supported.
- * -1 Error.
*/
-# define VIR_DRV_SUPPORTS_FEATURE(drv,conn,feature) \
- ((drv)->supports_feature ? (drv)->supports_feature((conn),(feature)) : 0)
+# define VIR_DRV_SUPPORTS_FEATURE(drv,conn,feature) \
+ ((drv)->supports_feature ? \
+ (drv)->supports_feature((conn), (feature)) > 0 : 0)
typedef virDrvOpenStatus
(*virDrvOpen) (virConnectPtr conn,
diff --git a/src/libvirt.c b/src/libvirt.c
index b4951c2..4188b45 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1605,7 +1605,10 @@ virDrvSupportsFeature (virConnectPtr conn, int feature)
return (-1);
}
- ret = VIR_DRV_SUPPORTS_FEATURE (conn->driver, conn, feature);
+ if (!conn->driver->supports_feature)
+ ret = 0;
+ else
+ ret = conn->driver->supports_feature(conn, feature);
if (ret < 0)
virDispatchError(conn);
--
1.7.3.2
8:59 a.m.
On 12/03/2010 01:48 AM, Jiri Denemark wrote:
virDrvSupportsFeature API is allowed to return -1 on error while all
but
one uses of VIR_DRV_SUPPORTS_FEATURE only check for (non)zero return
value. Let's make this macro return zero on error, which is what
everyone expects anyway.
---
src/driver.h | 8 ++++----
src/libvirt.c | 5 ++++-
2 files changed, 8 insertions(+), 5 deletions(-)
That is, if you care about the possibility of an error, call the
function directly; and if you use the macro, then you don't care about
errors and are happy treating an error the same as not present.
Seems reasonable.
diff --git a/src/driver.h b/src/driver.h
index b770e5e..e797a75 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -52,12 +52,12 @@ typedef enum {
* Note that you must check for errors.
However, this comment is no longer applicable. So you'll need a v2 that
fixes the documentation, and make it explicit that the macro ignores
errors, as well as a cross-reference to the actual function for someone
that cares about errors.
*
* Returns:
- * >= 1 Feature is supported.
+ * != 0 Feature is supported.
* 0 Feature is not supported.
- * -1 Error.
*/
-# define VIR_DRV_SUPPORTS_FEATURE(drv,conn,feature) \
- ((drv)->supports_feature ? (drv)->supports_feature((conn),(feature)) : 0)
+# define VIR_DRV_SUPPORTS_FEATURE(drv,conn,feature) \
+ ((drv)->supports_feature ? \
+ (drv)->supports_feature((conn), (feature)) > 0 : 0)
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
9:36 a.m.
> diff --git a/src/driver.h b/src/driver.h
> index b770e5e..e797a75 100644
> --- a/src/driver.h
> +++ b/src/driver.h
> @@ -52,12 +52,12 @@ typedef enum {
> * Note that you must check for errors.
However, this comment is no longer applicable. So you'll need a v2 that
fixes the documentation, and make it explicit that the macro ignores
errors, as well as a cross-reference to the actual function for someone
that cares about errors.
Ah, I haven't noticed that line... What about the following v2?
Jirka
From 592fd277d7a853025d92946701590485c0973205 Mon Sep 17 00:00:00 2001
Message-Id:
<592fd277d7a853025d92946701590485c0973205.1291390396.git.jdenemar(a)redhat.com>
From: Jiri Denemark <jdenemar(a)redhat.com>
Date: Fri, 3 Dec 2010 09:31:48 +0100
Subject: [PATCH] Change return value of VIR_DRV_SUPPORTS_FEATURE to bool
Mail-Followup-To: libvir-list(a)redhat.com
virDrvSupportsFeature API is allowed to return -1 on error while all but
one uses of VIR_DRV_SUPPORTS_FEATURE only check for (non)zero return
value. Let's make this macro return zero on error, which is what
everyone expects anyway.
---
src/driver.h | 15 +++++++++------
src/libvirt.c | 5 ++++-
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/driver.h b/src/driver.h
index b770e5e..75305fe 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -47,17 +47,20 @@ typedef enum {
/* Internal feature-detection macro. Don't call drv->supports_feature
- * directly, because it may be NULL, use this macro instead.
+ * directly if you don't have to, because it may be NULL, use this macro
+ * instead.
*
- * Note that you must check for errors.
+ * Note that this treats a possible error returned by drv->supports_feature
+ * the same as not supported. If you care about the error, call
+ * drv->supports_feature directly.
*
* Returns:
- * >= 1 Feature is supported.
+ * != 0 Feature is supported.
* 0 Feature is not supported.
- * -1 Error.
*/
-# define VIR_DRV_SUPPORTS_FEATURE(drv,conn,feature) \
- ((drv)->supports_feature ? (drv)->supports_feature((conn),(feature)) : 0)
+# define VIR_DRV_SUPPORTS_FEATURE(drv,conn,feature) \
+ ((drv)->supports_feature ? \
+ (drv)->supports_feature((conn), (feature)) > 0 : 0)
typedef virDrvOpenStatus
(*virDrvOpen) (virConnectPtr conn,
diff --git a/src/libvirt.c b/src/libvirt.c
index b4951c2..4188b45 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1605,7 +1605,10 @@ virDrvSupportsFeature (virConnectPtr conn, int feature)
return (-1);
}
- ret = VIR_DRV_SUPPORTS_FEATURE (conn->driver, conn, feature);
+ if (!conn->driver->supports_feature)
+ ret = 0;
+ else
+ ret = conn->driver->supports_feature(conn, feature);
if (ret < 0)
virDispatchError(conn);
--
1.7.3.2
9:38 a.m.
On 12/03/2010 08:36 AM, Jiri Denemark wrote:
>> diff --git a/src/driver.h b/src/driver.h
>> index b770e5e..e797a75 100644
>> --- a/src/driver.h
>> +++ b/src/driver.h
>> @@ -52,12 +52,12 @@ typedef enum {
>> * Note that you must check for errors.
>
> However, this comment is no longer applicable. So you'll need a v2 that
> fixes the documentation, and make it explicit that the macro ignores
> errors, as well as a cross-reference to the actual function for someone
> that cares about errors.
Ah, I haven't noticed that line... What about the following v2?
ACK.
/* Internal feature-detection macro. Don't call
drv->supports_feature
- * directly, because it may be NULL, use this macro instead.
+ * directly if you don't have to, because it may be NULL, use this macro
+ * instead.
*
- * Note that you must check for errors.
+ * Note that this treats a possible error returned by drv->supports_feature
+ * the same as not supported. If you care about the error, call
+ * drv->supports_feature directly.
*
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
5130
days inactive
5133
days old
4 comments
2 participants
participants (2)
-
Eric Blake -
Jiri Denemark