On 07/11/2012 07:35 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> This introduces a fairly basic reference counted virObject type
> and an associated virClass type, that use atomic operations for
> ref counting.
>
> In a global initializer (recommended to be invoked using the
> virOnceInit API), a virClass type must be allocated for each
> object type. This requires a class name, a "dispose" callback
> which will be invoked to free memory associated with the object's
> fields, and the size in bytes of the object struct.
>
> eg,
>
> virClassPtr connklass = virClassNew("virConnect",
> sizeof(virConnect),
> virConnectDispose);
I know why you use 'klass' instead of 'class' in isolation (for C++
compatibility), but when it is part of a longer word, it just looks weird.
> Object references can be manipulated with
>
> virObjectRef(conn)
> virObjectUnref(conn)
>
> The latter returns a true value, if the object has been
> freed (ie its ref count hit zero)
Should these return the resulting refcount, and/or add a query function
that tells the current ref count? There are some APIs where knowing how
many references remain may be useful, rather than just the boolean
information of being the last reference.
> +++ b/src/libvirt_private.syms
> @@ -1492,6 +1492,15 @@ nodeSuspendForDuration;
> virNodeSuspendGetTargetMask;
>
>
> +# virobject.h
> +virClassNew;
> +virClassName;
> +virObjectNew;
> +virObjectRef;
> +virObjectUnref;
> +virObjectIsClass;
Not sorted.
> +
> +struct _virClass {
> + unsigned int magic;
> + const char *name;
> + size_t objectSize;
> +
> + virObjectDisposeCallback dispose;
> +};
Meta-question - do we want to free _virClass objects on clean exit, to
keep valgrind analysis happy? Or are we okay with permanently
allocating them and leaking those references on exit? If the former,
then should _virClass be reference counted (that is, the _virClass
struct _also_ inherits from virObject, and each call to virObjectNew()
increments the class reference count, and each dispose decrements the
class reference count). Then again, it would make for an interesting
chicken-and-egg issue for setting up the _virClass class description for
each instance of a _virClass object.
> +void *virObjectNew(virClassPtr klass)
> +{
> + virObjectPtr obj = NULL;
> + char *somebytes;
> +
> + if (VIR_ALLOC_N(somebytes, klass->objectSize) < 0) {
> + virReportOOMError();
> + return NULL;
> + }
> + obj = (void*)somebytes;
This cast works, but I would have written:
obj = (virObjectPtr) somebytes;
> +
> +virClassPtr virClassNew(const char *name,
> + size_t objectSize,
> + virObjectDisposeCallback dispose)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);
Must the dispose callback be present, even if it has nothing to do?
ACK with nits fixed.
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list