virnetserverclient: Use automatic mutex management

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

src/rpc/virnetserverclient.c

@@ -234,14 +234,12 @@ int virNetServerClientAddFilter(virNetServerClient *client,
                                 virNetServerClientFilterFunc func,
                                 void *opaque)
 {
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
     virNetServerClientFilter *filter;
     virNetServerClientFilter **place;
-    int ret;
 
     filter = g_new0(virNetServerClientFilter, 1);
 
-    virObjectLock(client);
-
     filter->id = client->nextFilterID++;
     filter->func = func;
     filter->opaque = opaque;
@@ -251,21 +249,16 @@ int virNetServerClientAddFilter(virNetServerClient *client,
         place = &(*place)->next;
     *place = filter;
 
-    ret = filter->id;
+    return filter->id;
-
-    virObjectUnlock(client);
-
-    return ret;
 }
 
 void virNetServerClientRemoveFilter(virNetServerClient *client,
                                     int filterID)
 {
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
     virNetServerClientFilter *tmp;
     virNetServerClientFilter *prev;
 
-    virObjectLock(client);
-
     prev = NULL;
     tmp = client->filters;
     while (tmp) {
@@ -281,8 +274,6 @@ void virNetServerClientRemoveFilter(virNetServerClient *client,
         prev = tmp;
         tmp = tmp->next;
     }
-
-    virObjectUnlock(client);
 }
 
 
@@ -322,20 +313,20 @@ virNetServerClientCheckAccess(virNetServerClient *client)
 static void virNetServerClientDispatchMessage(virNetServerClient *client,
                                               virNetMessage *msg)
 {
-    virObjectLock(client);
+    VIR_WITH_OBJECT_LOCK_GUARD(client) {
         if (!client->dispatchFunc) {
             virNetMessageFree(msg);
             client->wantClose = true;
-        virObjectUnlock(client);
+            return;
-    } else {
+        }
-        virObjectUnlock(client);
+    }
+
     /* Accessing 'client' is safe, because virNetServerClientSetDispatcher
      * only permits setting 'dispatchFunc' once, so if non-NULL, it will
      * never change again
      */
     client->dispatchFunc(client, msg, client->dispatchOpaque);
 }
-}
 
 
 static void virNetServerClientSockTimerFunc(int timer,
@@ -343,13 +334,14 @@ static void virNetServerClientSockTimerFunc(int timer,
 {
     virNetServerClient *client = opaque;
     virNetMessage *msg = NULL;
-    virObjectLock(client);
+
+    VIR_WITH_OBJECT_LOCK_GUARD(client) {
         virEventUpdateTimeout(timer, -1);
         /* Although client->rx != NULL when this timer is enabled, it might have
          * changed since the client was unlocked in the meantime. */
         if (client->rx)
             msg = virNetServerClientDispatchRead(client);
-    virObjectUnlock(client);
+    }
 
     if (msg)
         virNetServerClientDispatchMessage(client, msg);
@@ -587,53 +579,45 @@ virJSONValue *virNetServerClientPreExecRestart(virNetServerClient *client)
     g_autoptr(virJSONValue) object = virJSONValueNewObject();
     g_autoptr(virJSONValue) sock = NULL;
     g_autoptr(virJSONValue) priv = NULL;
-
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
-    virObjectLock(client);
 
     if (virJSONValueObjectAppendNumberUlong(object, "id", client->id) < 0)
-        goto error;
+        return NULL;
     if (virJSONValueObjectAppendNumberInt(object, "auth", client->auth) < 0)
-        goto error;
+        return NULL;
     if (virJSONValueObjectAppendBoolean(object, "auth_pending", client->auth_pending) < 0)
-        goto error;
+        return NULL;
     if (virJSONValueObjectAppendBoolean(object, "readonly", client->readonly) < 0)
-        goto error;
+        return NULL;
     if (virJSONValueObjectAppendNumberUint(object, "nrequests_max", client->nrequests_max) < 0)
-        goto error;
+        return NULL;
 
     if (client->conn_time &&
         virJSONValueObjectAppendNumberLong(object, "conn_time",
                                            client->conn_time) < 0)
-        goto error;
+        return NULL;
 
     if (!(sock = virNetSocketPreExecRestart(client->sock)))
-        goto error;
+        return NULL;
 
     if (virJSONValueObjectAppend(object, "sock", &sock) < 0)
-        goto error;
+        return NULL;
 
     if (!(priv = client->privateDataPreExecRestart(client, client->privateData)))
-        goto error;
+        return NULL;
 
     if (virJSONValueObjectAppend(object, "privateData", &priv) < 0)
-        goto error;
-
-    virObjectUnlock(client);
-    return g_steal_pointer(&object);
-
- error:
-    virObjectUnlock(client);
         return NULL;
+
+    return g_steal_pointer(&object);
 }
 
 
 int virNetServerClientGetAuth(virNetServerClient *client)
 {
-    int auth;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
-    virObjectLock(client);
+
-    auth = client->auth;
+    return client->auth;
-    virObjectUnlock(client);
-    return auth;
 }
 
 
@@ -647,11 +631,9 @@ virNetServerClientSetAuthLocked(virNetServerClient *client,
 
 bool virNetServerClientGetReadonly(virNetServerClient *client)
 {
-    bool readonly;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
-    virObjectLock(client);
+
-    readonly = client->readonly;
+    return client->readonly;
-    virObjectUnlock(client);
-    return readonly;
 }
 
 
@@ -659,9 +641,9 @@ void
 virNetServerClientSetReadonly(virNetServerClient *client,
                               bool readonly)
 {
-    virObjectLock(client);
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
     client->readonly = readonly;
-    virObjectUnlock(client);
 }
 
 
@@ -677,52 +659,48 @@ long long virNetServerClientGetTimestamp(virNetServerClient *client)
 
 bool virNetServerClientHasTLSSession(virNetServerClient *client)
 {
-    bool has;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
-    virObjectLock(client);
+
-    has = client->tls ? true : false;
+    return !!client->tls;
-    virObjectUnlock(client);
-    return has;
 }
 
 
 virNetTLSSession *virNetServerClientGetTLSSession(virNetServerClient *client)
 {
-    virNetTLSSession *tls;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
-    virObjectLock(client);
+
-    tls = client->tls;
+    return client->tls;
-    virObjectUnlock(client);
-    return tls;
 }
 
 int virNetServerClientGetTLSKeySize(virNetServerClient *client)
 {
-    int size = 0;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
-    virObjectLock(client);
+
-    if (client->tls)
+    if (!client->tls)
-        size = virNetTLSSessionGetKeySize(client->tls);
+        return 0;
-    virObjectUnlock(client);
+
-    return size;
+    return virNetTLSSessionGetKeySize(client->tls);
 }
 
 int virNetServerClientGetFD(virNetServerClient *client)
 {
-    int fd = -1;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
-    virObjectLock(client);
+
-    if (client->sock)
+    if (!client->sock)
-        fd = virNetSocketGetFD(client->sock);
+        return -1;
-    virObjectUnlock(client);
+
-    return fd;
+    return virNetSocketGetFD(client->sock);
 }
 
 
 bool virNetServerClientIsLocal(virNetServerClient *client)
 {
-    bool local = false;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
-    virObjectLock(client);
+
-    if (client->sock)
+    if (!client->sock)
-        local = virNetSocketIsLocal(client->sock);
+        return false;
-    virObjectUnlock(client);
+
-    return local;
+    return virNetSocketIsLocal(client->sock);
 }
 
 
@@ -730,14 +708,12 @@ int virNetServerClientGetUNIXIdentity(virNetServerClient *client,
                                       uid_t *uid, gid_t *gid, pid_t *pid,
                                       unsigned long long *timestamp)
 {
-    int ret = -1;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
-    virObjectLock(client);
+
-    if (client->sock)
+    if (!client->sock)
-        ret = virNetSocketGetUNIXIdentity(client->sock,
+        return -1;
-                                          uid, gid, pid,
+
-                                          timestamp);
+    return virNetSocketGetUNIXIdentity(client->sock, uid, gid, pid, timestamp);
-    virObjectUnlock(client);
-    return ret;
 }
 
 
@@ -806,56 +782,60 @@ virNetServerClientCreateIdentity(virNetServerClient *client)
 
 virIdentity *virNetServerClientGetIdentity(virNetServerClient *client)
 {
-    virIdentity *ret = NULL;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
-    virObjectLock(client);
+
     if (!client->identity)
         client->identity = virNetServerClientCreateIdentity(client);
-    if (client->identity)
+
-        ret = g_object_ref(client->identity);
+    if (!client->identity)
-    virObjectUnlock(client);
+        return NULL;
-    return ret;
+
+    return g_object_ref(client->identity);
 }
 
 
 void virNetServerClientSetIdentity(virNetServerClient *client,
                                    virIdentity *identity)
 {
-    virObjectLock(client);
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
     g_clear_object(&client->identity);
     client->identity = identity;
     if (client->identity)
         g_object_ref(client->identity);
-    virObjectUnlock(client);
 }
 
 
 int virNetServerClientGetSELinuxContext(virNetServerClient *client,
                                         char **context)
 {
-    int ret = 0;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
     *context = NULL;
-    virObjectLock(client);
+
-    if (client->sock)
+    if (!client->sock)
-        ret = virNetSocketGetSELinuxContext(client->sock, context);
+        return 0;
-    virObjectUnlock(client);
+
-    return ret;
+    return virNetSocketGetSELinuxContext(client->sock, context);
 }
 
 
 bool virNetServerClientIsSecure(virNetServerClient *client)
 {
-    bool secure = false;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
-    virObjectLock(client);
+
     if (client->tls)
-        secure = true;
+        return true;
+
 #if WITH_SASL
     if (client->sasl)
-        secure = true;
+        return true;
 #endif
+
     if (client->sock && virNetSocketIsLocal(client->sock))
-        secure = true;
+        return true;
-    virObjectUnlock(client);
+
-    return secure;
+    return false;
 }
 
 
@@ -863,53 +843,47 @@ bool virNetServerClientIsSecure(virNetServerClient *client)
 void virNetServerClientSetSASLSession(virNetServerClient *client,
                                       virNetSASLSession *sasl)
 {
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
     /* We don't set the sasl session on the socket here
      * because we need to send out the auth confirmation
      * in the clear. Only once we complete the next 'tx'
      * operation do we switch to SASL mode
      */
-    virObjectLock(client);
     client->sasl = virObjectRef(sasl);
-    virObjectUnlock(client);
 }
 
 
 virNetSASLSession *virNetServerClientGetSASLSession(virNetServerClient *client)
 {
-    virNetSASLSession *sasl;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
-    virObjectLock(client);
+
-    sasl = client->sasl;
+    return client->sasl;
-    virObjectUnlock(client);
-    return sasl;
 }
 
 bool virNetServerClientHasSASLSession(virNetServerClient *client)
 {
-    bool has = false;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
-    virObjectLock(client);
+
-    has = !!client->sasl;
+    return !!client->sasl;
-    virObjectUnlock(client);
-    return has;
 }
 #endif
 
 
 void *virNetServerClientGetPrivateData(virNetServerClient *client)
 {
-    void *data;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
-    virObjectLock(client);
+
-    data = client->privateData;
+    return client->privateData;
-    virObjectUnlock(client);
-    return data;
 }
 
 
 void virNetServerClientSetCloseHook(virNetServerClient *client,
                                     virNetServerClientCloseFunc cf)
 {
-    virObjectLock(client);
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
     client->privateDataCloseFunc = cf;
-    virObjectUnlock(client);
 }
 
 
@@ -917,7 +891,8 @@ void virNetServerClientSetDispatcher(virNetServerClient *client,
                                      virNetServerClientDispatchFunc func,
                                      void *opaque)
 {
-    virObjectLock(client);
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
     /* Only set dispatcher if not already set, to avoid race
      * with dispatch code that runs without locks held
      */
@@ -925,7 +900,6 @@ void virNetServerClientSetDispatcher(virNetServerClient *client,
         client->dispatchFunc = func;
         client->dispatchOpaque = opaque;
     }
-    virObjectUnlock(client);
 }
 
 
@@ -1042,9 +1016,9 @@ virNetServerClientCloseLocked(virNetServerClient *client)
 void
 virNetServerClientClose(virNetServerClient *client)
 {
-    virObjectLock(client);
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
     virNetServerClientCloseLocked(client);
-    virObjectUnlock(client);
 }
 
 
@@ -1057,16 +1031,16 @@ virNetServerClientIsClosedLocked(virNetServerClient *client)
 
 void virNetServerClientDelayedClose(virNetServerClient *client)
 {
-    virObjectLock(client);
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
     client->delayedClose = true;
-    virObjectUnlock(client);
 }
 
 void virNetServerClientImmediateClose(virNetServerClient *client)
 {
-    virObjectLock(client);
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
     client->wantClose = true;
-    virObjectUnlock(client);
 }
 
 
@@ -1079,26 +1053,26 @@ virNetServerClientWantCloseLocked(virNetServerClient *client)
 
 int virNetServerClientInit(virNetServerClient *client)
 {
-    virObjectLock(client);
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+    int ret = -1;
 
     if (!client->tlsCtxt) {
         /* Plain socket, so prepare to read first message */
         if (virNetServerClientRegisterEvent(client) < 0)
             goto error;
-    } else {
+        return 0;
-        int ret;
+    }
 
-        if (!(client->tls = virNetTLSSessionNew(client->tlsCtxt,
+    if (!(client->tls = virNetTLSSessionNew(client->tlsCtxt, NULL)))
-                                                NULL)))
         goto error;
 
-        virNetSocketSetTLSSession(client->sock,
+    virNetSocketSetTLSSession(client->sock, client->tls);
-                                  client->tls);
 
     /* Begin the TLS handshake. */
-        virObjectLock(client->tlsCtxt);
+    VIR_WITH_OBJECT_LOCK_GUARD(client->tlsCtxt) {
         ret = virNetTLSSessionHandshake(client->tls);
-        virObjectUnlock(client->tlsCtxt);
+    }
+
     if (ret == 0) {
         /* Unlikely, but ...  Next step is to check the certificate. */
         if (virNetServerClientCheckAccess(client) < 0)
@@ -1114,14 +1088,11 @@ int virNetServerClientInit(virNetServerClient *client)
     } else {
         goto error;
     }
-    }
 
-    virObjectUnlock(client);
     return 0;
 
  error:
     client->wantClose = true;
-    virObjectUnlock(client);
     return -1;
 }
 
@@ -1406,11 +1377,13 @@ virNetServerClientDispatchWrite(virNetServerClient *client)
 static void
 virNetServerClientDispatchHandshake(virNetServerClient *client)
 {
-    int ret;
+    int ret = -1;
+
     /* Continue the handshake. */
-    virObjectLock(client->tlsCtxt);
+    VIR_WITH_OBJECT_LOCK_GUARD(client->tlsCtxt) {
         ret = virNetTLSSessionHandshake(client->tls);
-    virObjectUnlock(client->tlsCtxt);
+    }
+
     if (ret == 0) {
         /* Finished.  Next step is to check the certificate. */
         if (virNetServerClientCheckAccess(client) < 0)
@@ -1435,16 +1408,13 @@ virNetServerClientDispatchEvent(virNetSocket *sock, int events, void *opaque)
     virNetServerClient *client = opaque;
     virNetMessage *msg = NULL;
 
-    virObjectLock(client);
+    VIR_WITH_OBJECT_LOCK_GUARD(client) {
-
         if (client->sock != sock) {
             virNetSocketRemoveIOCallback(sock);
-        virObjectUnlock(client);
             return;
         }
 
-    if (events & (VIR_EVENT_HANDLE_WRITABLE |
+        if (events & (VIR_EVENT_HANDLE_WRITABLE | VIR_EVENT_HANDLE_READABLE)) {
-                  VIR_EVENT_HANDLE_READABLE)) {
             if (client->tls &&
                 virNetTLSSessionGetHandshakeStatus(client->tls) !=
                 VIR_NET_TLS_HANDSHAKE_COMPLETE) {
@@ -1452,19 +1422,15 @@ virNetServerClientDispatchEvent(virNetSocket *sock, int events, void *opaque)
             } else {
                 if (events & VIR_EVENT_HANDLE_WRITABLE)
                     virNetServerClientDispatchWrite(client);
-            if (events & VIR_EVENT_HANDLE_READABLE &&
+                if ((events & VIR_EVENT_HANDLE_READABLE) && client->rx)
-                client->rx)
                     msg = virNetServerClientDispatchRead(client);
             }
         }
 
-    /* NB, will get HANGUP + READABLE at same time upon
+        /* NB, will get HANGUP + READABLE at same time upon disconnect */
-     * disconnect */
+        if (events & (VIR_EVENT_HANDLE_ERROR | VIR_EVENT_HANDLE_HANGUP))
-    if (events & (VIR_EVENT_HANDLE_ERROR |
-                  VIR_EVENT_HANDLE_HANGUP))
             client->wantClose = true;
-
+    }
-    virObjectUnlock(client);
 
     if (msg)
         virNetServerClientDispatchMessage(client, msg);
@@ -1499,24 +1465,18 @@ virNetServerClientSendMessageLocked(virNetServerClient *client,
 int virNetServerClientSendMessage(virNetServerClient *client,
                                   virNetMessage *msg)
 {
-    int ret;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
 
-    virObjectLock(client);
+    return virNetServerClientSendMessageLocked(client, msg);
-    ret = virNetServerClientSendMessageLocked(client, msg);
-    virObjectUnlock(client);
-
-    return ret;
 }
 
 
 bool
 virNetServerClientIsAuthenticated(virNetServerClient *client)
 {
-    bool authenticated;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
-    virObjectLock(client);
+
-    authenticated = virNetServerClientAuthMethodImpliesAuthenticated(client->auth);
+    return virNetServerClientAuthMethodImpliesAuthenticated(client->auth);
-    virObjectUnlock(client);
-    return authenticated;
 }
 
 
@@ -1556,57 +1516,44 @@ virNetServerClientInitKeepAlive(virNetServerClient *client,
                                 int interval,
                                 unsigned int count)
 {
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
     virKeepAlive *ka;
-    int ret = -1;
-
-    virObjectLock(client);
 
     if (!(ka = virKeepAliveNew(interval, count, client,
                                virNetServerClientKeepAliveSendCB,
                                virNetServerClientKeepAliveDeadCB,
                                virObjectFreeCallback)))
-        goto cleanup;
+        return -1;
+
     /* keepalive object has a reference to client */
     virObjectRef(client);
 
     client->keepalive = ka;
-    ret = 0;
+    return 0;
- cleanup:
-    virObjectUnlock(client);
-
-    return ret;
 }
 
 int
 virNetServerClientStartKeepAlive(virNetServerClient *client)
 {
-    int ret = -1;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
-
-    virObjectLock(client);
 
     /* The connection might have been closed before we got here and thus the
      * keepalive object could have been removed too.
      */
     if (!client->keepalive) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
-                       _("connection not open"));
+        return -1;
-        goto cleanup;
     }
 
-    ret = virKeepAliveStart(client->keepalive, 0, 0);
+    return virKeepAliveStart(client->keepalive, 0, 0);
-
- cleanup:
-    virObjectUnlock(client);
-    return ret;
 }
 
 int
 virNetServerClientGetTransport(virNetServerClient *client)
 {
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
     int ret = -1;
 
-    virObjectLock(client);
-
     if (client->sock && virNetSocketIsLocal(client->sock))
         ret = VIR_CLIENT_TRANS_UNIX;
     else
@@ -1615,8 +1562,6 @@ virNetServerClientGetTransport(virNetServerClient *client)
     if (client->tls)
         ret = VIR_CLIENT_TRANS_TLS;
 
-    virObjectUnlock(client);
-
     return ret;
 }
 
@@ -1625,16 +1570,15 @@ virNetServerClientGetInfo(virNetServerClient *client,
                           bool *readonly, char **sock_addr,
                           virIdentity **identity)
 {
-    int ret = -1;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(client);
     const char *addr;
 
-    virObjectLock(client);
     *readonly = client->readonly;
 
     if (!(addr = virNetServerClientRemoteAddrStringURI(client))) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("No network socket associated with client"));
-        goto cleanup;
+        return -1;
     }
 
     *sock_addr = g_strdup(addr);
@@ -1642,15 +1586,11 @@ virNetServerClientGetInfo(virNetServerClient *client,
     if (!client->identity) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("No identity information available for client"));
-        goto cleanup;
+        return -1;
     }
 
     *identity = g_object_ref(client->identity);
-
+    return 0;
-    ret = 0;
- cleanup:
-    virObjectUnlock(client);
-    return ret;
 }