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

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



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 <jferlan redhat com>
> ---
>  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 :|


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