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

Re: [libvirt] [PATCH v3 02/10] Convert public datatypes to inherit from virObject



On 08/06/2012 05:52 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
> 
> This converts the following public API datatypes to use the
> virObject infrastructure:

> +virGetConnect(void)
> +{
>      virConnectPtr ret;
>  
> -    if (VIR_ALLOC(ret) < 0) {
> -        virReportOOMError();
> -        goto failed;
> -    }
> +    if (virDataTypesInitialize() < 0)
> +        return NULL;
> +
> +    if (!(ret = virObjectNew(virConnectClass)))
> +        return NULL;
> +
>      if (virMutexInit(&ret->lock) < 0) {
>          VIR_FREE(ret);
> -        goto failed;
> +        return NULL;

Theoretically, we have left non-poisoned memory on the heap, and
careless memory management could happen to revive the pointer and then
operate on it; but as that would imply a chain of multiple bugs, I'm not
too worried about the use of VIR_FREE(ret) here instead.  Poisoning
before free() is only a debug aid, after all.


> +# define VIR_IS_CONNECT(obj) \
> +    (virObjectIsClass((obj), virConnectClass))

Parens around 'obj' are redundant in this particular context, but it's
not a bad habit to have in general when writing macros, so I don't care
if you leave them.

> +
> +# define VIR_IS_DOMAIN(obj) \
> +    (virObjectIsClass((obj), virDomainClass))
> +# define VIR_IS_CONNECTED_DOMAIN(obj) \
> +    (VIR_IS_DOMAIN(obj) && VIR_IS_CONNECT((obj)->conn))

For example, in _this_ macro, the parens around the second 'obj' are
mandatory.

> @@ -1429,14 +1429,16 @@ error:
>   * matching virConnectClose, and all other references will be released
>   * after the corresponding operation completes.
>   *
> - * Returns the number of remaining references on success
> - * (positive implies that some other call still has a reference open,
> - * 0 implies that no references remain and the connection is closed),
> - * or -1 on failure.  It is possible for the last virConnectClose to
> - * return a positive value if some other object still has a temporary
> - * reference to the connection, but the application should not try to
> - * further use a connection after the virConnectClose that matches the
> - * initial open.
> + * Returns a positive number if at least 1 reference remains on
> + * success. The returned value should not be assumed to be the total
> + * reference count. A return of 0 implies no references remain and
> + * the connection is closed & memory has been freed. A return of -1

s/&/and/ - no need to abbreviate in formal documentation.

ACK with the doc fix.

-- 
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]