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

Re: [libvirt] [PATCH 05/13] Convert public datatypes to inherit from virObject



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

> @@ -426,6 +316,9 @@ virInterfacePtr
>  virGetInterface(virConnectPtr conn, const char *name, const char *mac) {

Resuming where I left off...


> @@ -558,7 +389,9 @@ virStoragePoolPtr
>  virGetStoragePool(virConnectPtr conn, const char *name,
>                    const unsigned char *uuid) {
>      virStoragePoolPtr ret = NULL;
> -    char uuidstr[VIR_UUID_STRING_BUFLEN];
> +
> +    if (virDataTypesInitialize() < 0)
> +        return NULL;

Another instance where we should either remove the uuidstr
independently, or start logging it to make the logs more useful.

>  virStreamPtr virGetStream(virConnectPtr conn) {

As long as you are touching this area of code, is it worth giving this
function consistent formatting?

virStreamPtr
virGetStream(virConnectPtr conn)
{

> +++ b/src/datatypes.h

> +# define VIR_IS_NETWORK(obj) \
> +    (virObjectIsClass((virObjectPtr)obj, virNetworkClass))

Technically under-parenthesized, should be:

(virObjectIsClass((virObjectPtr)(obj), virNetworkClass))

in case 'obj' expands to a non-trivial expression.  (Then again, our
likelihood of using this macro on a non-trivial expression is
practically zero).  Same goes for all the other VIR_IS_* macros wrapping
virObjectIsClass.

> +++ b/src/libvirt.c

> @@ -1430,10 +1430,9 @@ virConnectClose(virConnectPtr conn)
>          goto error;
>      }
>  
> -    ret = virUnrefConnect(conn);
> -    if (ret < 0)
> -        goto error;
> -    return ret;
> +    if (!virObjectUnref(conn))
> +        return 0;
> +    return 1;

Here is where you are changing the return value; you need to update the
documentation to match.

> +++ b/src/libvirt_private.syms

> +virSecretClass;
> +virStreamClass;
> +virStorageVolClass;
> +virStoragePoolClass;

Not quite sorted.

Mostly mechanical, and overall a nice patch, but I did point out one or
two issues worth fixing for v2 (at which point, I will just review the
interdiff).

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