Re: [PATCH v3 06/10] Turn virNetSASLContext and virNetSASLSession into virObject instances

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

 



On 08/06/2012 05:53 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> 
> Make virNetSASLContext and virNetSASLSession use virObject APIs
> for reference counting
> 
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
>  daemon/remote.c              |    8 ++--
>  src/remote/remote_driver.c   |    4 +-
>  src/rpc/virnetclient.c       |    7 ++-
>  src/rpc/virnetsaslcontext.c  |  106 ++++++++++++++++++------------------------
>  src/rpc/virnetsaslcontext.h  |    8 +---
>  src/rpc/virnetserverclient.c |    7 ++-
>  src/rpc/virnetsocket.c       |    7 ++-
>  7 files changed, 61 insertions(+), 86 deletions(-)

Not as dramatic, but still a net reduction.

Fails 'make check', and missing changes to cfg.mk to clean up the cruft.

> +++ b/src/rpc/virnetclient.c
> @@ -497,7 +497,7 @@ void virNetClientFree(virNetClientPtr client)
>      virNetSocketFree(client->sock);
>      virObjectUnref(client->tls);
>  #if HAVE_SASL
> -    virNetSASLSessionFree(client->sasl);
> +    virObjectUnref(client->sasl);
>  #endif

A followup patch could probably remove this #ifdef protection, if
client->sasl exists but is NULL when HAVE_SASL is not present.

> @@ -152,28 +176,11 @@ cleanup:
>  }
>  
>  
> -void virNetSASLContextRef(virNetSASLContextPtr ctxt)
> -{
> -    virMutexLock(&ctxt->lock);
> -    ctxt->refs++;
> -    virMutexUnlock(&ctxt->lock);
> -}
> -
> -void virNetSASLContextFree(virNetSASLContextPtr ctxt)
> +void virNetSASLContextDispose(void *obj)

For consistency with the prototype earlier in the file, I would have
marked this static, but what you have works.

> @@ -186,10 +193,8 @@ virNetSASLSessionPtr virNetSASLSessionNewClient(virNetSASLContextPtr ctxt ATTRIB
>      virNetSASLSessionPtr sasl = NULL;
>      int err;
>  
> -    if (VIR_ALLOC(sasl) < 0) {
> -        virReportOOMError();
> -        goto cleanup;
> -    }
> +    if (!(sasl = virObjectNew(virNetSASLSessionClass)))
> +        return NULL;

I suppose that since this function takes a virNetSASLContextPtr, and
creating one of those already calls virNetSASLContextInitialize(), that
you are safe not calling the one-shot initialization here after all.

> @@ -231,10 +235,8 @@ virNetSASLSessionPtr virNetSASLSessionNewServer(virNetSASLContextPtr ctxt ATTRIB
>      virNetSASLSessionPtr sasl = NULL;
>      int err;
>  
> -    if (VIR_ALLOC(sasl) < 0) {
> -        virReportOOMError();
> -        goto cleanup;
> -    }
> +    if (!(sasl = virObjectNew(virNetSASLSessionClass)))
> +        return NULL;

Another instance of the same comment.

ACK with this squashed in (oh, one of those cfg.mk tweaks actually
belongs in 5/10, and you may want to factor out the duplication cleanup
between libvirt_{private,sasl}.syms into a separate patch):

diff --git i/cfg.mk w/cfg.mk
index ccff146..dc39646 100644
--- i/cfg.mk
+++ w/cfg.mk
@@ -156,9 +156,6 @@ useless_free_options =				\
   --name=virNetServerProgramFree                \
   --name=virNetServerServiceFree                \
   --name=virNetSocketFree                       \
-  --name=virNetSASLContextFree                  \
-  --name=virNetSASLSessionFree                  \
-  --name=virNetTLSSessionFree                   \
   --name=virNWFilterDefFree			\
   --name=virNWFilterEntryFree			\
   --name=virNWFilterHashTableFree		\
diff --git i/src/libvirt_private.syms w/src/libvirt_private.syms
index acaa6f3..c0bb5a5 100644
--- i/src/libvirt_private.syms
+++ w/src/libvirt_private.syms
@@ -1470,27 +1470,13 @@ xdr_virNetMessageError;


 # virnetsaslcontext.h
-virNetSASLContextCheckIdentity;
-virNetSASLContextFree;
 virNetSASLContextNewClient;
-virNetSASLContextNewServer;
-virNetSASLContextRef;
 virNetSASLSessionClientStart;
 virNetSASLSessionClientStep;
 virNetSASLSessionDecode;
 virNetSASLSessionEncode;
-virNetSASLSessionExtKeySize;
-virNetSASLSessionFree;
-virNetSASLSessionGetIdentity;
-virNetSASLSessionGetKeySize;
 virNetSASLSessionGetMaxBufSize;
-virNetSASLSessionListMechanisms;
 virNetSASLSessionNewClient;
-virNetSASLSessionNewServer;
-virNetSASLSessionRef;
-virNetSASLSessionSecProps;
-virNetSASLSessionServerStart;
-virNetSASLSessionServerStep;


 # virnetserver.h
@@ -1542,7 +1528,6 @@ virNetServerClientSetCloseHook;
 virNetServerClientSetDispatcher;
 virNetServerClientSetIdentity;
 virNetServerClientSetPrivateData;
-virNetServerClientSetSASLSession;
 virNetServerClientStartKeepAlive;
 virNetServerClientWantClose;

diff --git i/src/libvirt_sasl.syms w/src/libvirt_sasl.syms
index 2c278c8..cc46c0d 100644
--- i/src/libvirt_sasl.syms
+++ w/src/libvirt_sasl.syms
@@ -6,7 +6,6 @@
 virNetSASLContextCheckIdentity;
 virNetSASLContextNewServer;
 virNetSASLSessionExtKeySize;
-virNetSASLSessionFree;
 virNetSASLSessionGetIdentity;
 virNetSASLSessionGetKeySize;
 virNetSASLSessionListMechanisms;


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

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