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

6 Aug 2012

On 08/06/2012 05:53 AM, Daniel P. Berrange wrote:
...
From: "Daniel P. Berrange" <berrange@redhat.com>
Make virNetSASLContext and virNetSASLSession use virObject APIs
for reference counting
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 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@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

    