Re: [libvirt] [PATCH] Call private virUnref*() functions from hashtable item destructors

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

 



On Thu, May 07, 2009 at 02:05:06PM -0400, Laine Stump wrote:
> Something like this?
> 
> Do you think that the virUnref*() functions should continue to error out
> if the conn is invalid? Or should they maybe just refrain from grabbing
> the conn mutex, but still decrement the ref count and release the object
> when necessary?

I think this patch is fine. The reason the public APIs do error checking
on the connection object, is because we dont entirely trust the source.
In this scenario, the objects are coming straight out of our internal
hash table, and if we can't trust that then we're so badly doomed we've
got bigger things to worry about :-)  

So, ACK to this patch

> (P.S. Is this patch mail formatted correctly? I'm doing this as much to
> get the procedure correct as to fix the code...)

Looks fine to me. I prefer inline patches :-)

> 
> ---
>  src/datatypes.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/datatypes.c b/src/datatypes.c
> index b1013f2..eceb839 100644
> --- a/src/datatypes.c
> +++ b/src/datatypes.c
> @@ -49,7 +49,7 @@
>  static int
>  virDomainFreeName(virDomainPtr domain, const char *name ATTRIBUTE_UNUSED)
>  {
> -    return (virDomainFree(domain));
> +    return (virUnrefDomain(domain));
>  }
>  
>  /**
> @@ -63,7 +63,7 @@ virDomainFreeName(virDomainPtr domain, const char *name ATTRIBUTE_UNUSED)
>  static int
>  virNetworkFreeName(virNetworkPtr network, const char *name ATTRIBUTE_UNUSED)
>  {
> -    return (virNetworkFree(network));
> +    return (virUnrefNetwork(network));
>  }
>  
>  /**
> @@ -77,7 +77,7 @@ virNetworkFreeName(virNetworkPtr network, const char *name ATTRIBUTE_UNUSED)
>  static int
>  virStoragePoolFreeName(virStoragePoolPtr pool, const char *name ATTRIBUTE_UNUSED)
>  {
> -    return (virStoragePoolFree(pool));
> +    return (virUnrefStoragePool(pool));
>  }
>  
>  /**
> @@ -91,7 +91,7 @@ virStoragePoolFreeName(virStoragePoolPtr pool, const char *name ATTRIBUTE_UNUSED
>  static int
>  virStorageVolFreeName(virStorageVolPtr vol, const char *name ATTRIBUTE_UNUSED)
>  {
> -    return (virStorageVolFree(vol));
> +    return (virUnrefStorageVol(vol));
>  }
>  
>  /**
> -- 
> 1.6.0.6
> 
> --
> Libvir-list mailing list
> Libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]