Re: [libvirt] [PATCH 4/7] util: Introduce virObjectGetRWLockableObj

[Daniel P. Berrange]

On Fri, Jul 28, 2017 at 12:38:58PM -0400, John Ferlan wrote:
> Introduce a helper to handle the error path more cleanly. The same
> as virObjectGetLockableObj.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/util/virobject.c | 51 ++-
>  1 file changed, 30 insertions(+), 21 deletions(-)
> 
> diff --git a/src/util/virobject.c b/src/util/virobject.c
> index f9047a1..0db98c3 100644
> --- a/src/util/virobject.c
> +++ b/src/util/virobject.c
> @@ -383,6 +383,22 @@ virObjectGetLockableObj(void *anyobj)
>  }
>  
>  
> +static virObjectRWLockablePtr
> +virObjectGetRWLockableObj(void *anyobj)
> +{
> +virObjectPtr obj;
> +
> +if (virObjectIsClass(anyobj, virObjectRWLockableClass))
> +return anyobj;
> +
> +obj = anyobj;
> +VIR_WARN("Object %p (%s) is not a virObjectRWLockable instance",
> +  anyobj, obj ? obj->klass->name : "(unknown)");
> +
> +return NULL;
> +}
> +
> +
>  /**
>   * virObjectLock:
>   * @anyobj: any instance of virObjectLockable or virObjectRWLockable
> @@ -422,18 +438,13 @@ virObjectLock(void *anyobj)
>  int
>  virObjectLockRead(void *anyobj)
>  {
> -if (virObjectIsClass(anyobj, virObjectRWLockableClass)) {
> -virObjectRWLockablePtr obj = anyobj;
> -virRWLockRead(&obj->lock);
> -return 0;
> -} else {
> -virObjectPtr obj = anyobj;
> -VIR_WARN("Object %p (%s) is not a virObjectRWLockable instance",
> - anyobj, obj ? obj->klass->name : "(unknown)");
> -virReportError(VIR_ERR_INTERNAL_ERROR,
> -   _("unable to obtain rwlock for object=%p"), anyobj);
> -}
> -return -1;
> +virObjectRWLockablePtr obj = virObjectGetRWLockableObj(anyobj);
> +
> +if (!obj)
> +return -1;
> +
> +virRWLockRead(&obj->lock);
> +return 0;
>  }
>  
>  
> @@ -454,18 +465,16 @@ virObjectLockRead(void *anyobj)
>  int
>  virObjectLockWrite(void *anyobj)
>  {
> -if (virObjectIsClass(anyobj, virObjectRWLockableClass)) {
> -virObjectRWLockablePtr obj = anyobj;
> -virRWLockWrite(&obj->lock);
> -return 0;
> -} else {
> -virObjectPtr obj = anyobj;
> -VIR_WARN("Object %p (%s) is not a virObjectRWLockable instance",
> - anyobj, obj ? obj->klass->name : "(unknown)");
> +virObjectRWLockablePtr obj = virObjectGetRWLockableObj(anyobj);
> +
> +if (!obj) {
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> _("unable to obtain rwlock for object=%p"), anyobj);
> +return -1;
>  }
> -return -1;
> +
> +virRWLockWrite(&obj->lock);
> +return 0;
>  }

I don't really thing this is a good idea for reasons outlined in the
replies to earlier patches.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 4/7] util: Introduce virObjectGetRWLockableObj

[Pavel Hrdina]

On Fri, Jul 28, 2017 at 12:38:58PM -0400, John Ferlan wrote:
> Introduce a helper to handle the error path more cleanly. The same
> as virObjectGetLockableObj.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/util/virobject.c | 51 ++-
>  1 file changed, 30 insertions(+), 21 deletions(-)

Without the return value as for patch 01 and 02

Reviewed-by: Pavel Hrdina 


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 4/7] util: Introduce virObjectGetRWLockableObj

[John Ferlan]

Introduce a helper to handle the error path more cleanly. The same
as virObjectGetLockableObj.

Signed-off-by: John Ferlan 
---
 src/util/virobject.c | 51 ++-
 1 file changed, 30 insertions(+), 21 deletions(-)

diff --git a/src/util/virobject.c b/src/util/virobject.c
index f9047a1..0db98c3 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -383,6 +383,22 @@ virObjectGetLockableObj(void *anyobj)
 }
 
 
+static virObjectRWLockablePtr
+virObjectGetRWLockableObj(void *anyobj)
+{
+virObjectPtr obj;
+
+if (virObjectIsClass(anyobj, virObjectRWLockableClass))
+return anyobj;
+
+obj = anyobj;
+VIR_WARN("Object %p (%s) is not a virObjectRWLockable instance",
+  anyobj, obj ? obj->klass->name : "(unknown)");
+
+return NULL;
+}
+
+
 /**
  * virObjectLock:
  * @anyobj: any instance of virObjectLockable or virObjectRWLockable
@@ -422,18 +438,13 @@ virObjectLock(void *anyobj)
 int
 virObjectLockRead(void *anyobj)
 {
-if (virObjectIsClass(anyobj, virObjectRWLockableClass)) {
-virObjectRWLockablePtr obj = anyobj;
-virRWLockRead(&obj->lock);
-return 0;
-} else {
-virObjectPtr obj = anyobj;
-VIR_WARN("Object %p (%s) is not a virObjectRWLockable instance",
- anyobj, obj ? obj->klass->name : "(unknown)");
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("unable to obtain rwlock for object=%p"), anyobj);
-}
-return -1;
+virObjectRWLockablePtr obj = virObjectGetRWLockableObj(anyobj);
+
+if (!obj)
+return -1;
+
+virRWLockRead(&obj->lock);
+return 0;
 }
 
 
@@ -454,18 +465,16 @@ virObjectLockRead(void *anyobj)
 int
 virObjectLockWrite(void *anyobj)
 {
-if (virObjectIsClass(anyobj, virObjectRWLockableClass)) {
-virObjectRWLockablePtr obj = anyobj;
-virRWLockWrite(&obj->lock);
-return 0;
-} else {
-virObjectPtr obj = anyobj;
-VIR_WARN("Object %p (%s) is not a virObjectRWLockable instance",
- anyobj, obj ? obj->klass->name : "(unknown)");
+virObjectRWLockablePtr obj = virObjectGetRWLockableObj(anyobj);
+
+if (!obj) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("unable to obtain rwlock for object=%p"), anyobj);
+return -1;
 }
-return -1;
+
+virRWLockWrite(&obj->lock);
+return 0;
 }
 
 
-- 
2.9.4

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