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

Re: [libvirt] [PATCH 2/7] Add a virObjectLockable class holding a mutex



On 01/11/2013 05:13 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
> 
> A great many virObject instances require a mutex, so introduce
> a convenient class for this which provides a mutex. This avoids
> repeating the tedious init/destroy code
> ---
>  src/libvirt_private.syms |  4 +++
>  src/util/virobject.c     | 87 ++++++++++++++++++++++++++++++++++++++++++++++--
>  src/util/virobject.h     | 20 +++++++++++
>  3 files changed, 109 insertions(+), 2 deletions(-)


> +void *virObjectLockableNew(virClassPtr klass)
> +{
> +    virObjectLockablePtr obj;
> +
> +    if (!virClassIsDerivedFrom(klass, virClassForObjectLockable())) {
> +        virReportInvalidArg(klass,
> +                            _("Class %s must derive from virObjectLockable"),
> +                            virClassName(klass));
> +        return NULL;
> +    }
> +
> +    if (!(obj = virObjectNew(klass)))
> +        return NULL;
> +
> +    if (virMutexInit(&obj->lock) < 0) {
> +        virReportSystemError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                             _("Unable to initialize mutex"));
> +        virObjectUnref(obj);
> +        return NULL;
> +    }

This creates the new object locked...

> +
> +    return obj;
> +}
> +
> +
> +static void virObjectLockableDispose(void *anyobj)
> +{
> +    virObjectLockablePtr obj = anyobj;
> +
> +    virMutexDestroy(&obj->lock);

...but this doesn't unlock anything.  You may want to document that the
user is required to unlock the object before losing the last reference.

> + * virObjectLock:
> + * @anyobj: any instance of virObjectLockablePtr
> + *
> + * Acquire a lock on @anyobj. The lock must be
> + * released by virObjectUnlock.
> + */
> +void virObjectLock(void *anyobj)
> +{
> +    virObjectLockablePtr obj = anyobj;

Is it worth a sanity check that anyobj is actually an object of the
right class, before we blindly dereference something wrong due to a
coding error?

> +void virObjectUnlock(void *anyobj)
> +{
> +    virObjectLockablePtr obj = anyobj;
> +

And again, would a sanity check help?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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