Re: [libvirt] [PATCH 2/5] rpc: simplify calling convention of virNetServerClientDispatchFunc

Currently virNetServerClientDispatchFunc implementations are only responsible for free'ing the "msg" parameter upon success. Simplify the calling convention by making it their unconditional responsibility to free the "msg", and close the client if desired. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/rpc/virnetserver.c | 24 +++++++++++++----------- src/rpc/virnetserverclient.c | 6 +++--- src/rpc/virnetserverclient.h | 9 ++++++--- 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 28afe54e49..7a1376bf49 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -182,15 +182,14 @@ static void virNetServerHandleJob(void *jobOpaque, void *opaque) VIR_FREE(job); } -static int virNetServerDispatchNewMessage(virNetServerClientPtr client, - virNetMessagePtr msg, - void *opaque) +static void virNetServerDispatchNewMessage(virNetServerClientPtr client, + virNetMessagePtr msg, + void *opaque) { virNetServerPtr srv = opaque; virNetServerProgramPtr prog = NULL; unsigned int priority = 0; size_t i; - int ret = -1; VIR_DEBUG("server=%p client=%p message=%p", srv, client, msg); @@ -207,7 +206,7 @@ static int virNetServerDispatchNewMessage(virNetServerClientPtr client, virNetServerJobPtr job; if (VIR_ALLOC(job) < 0) - goto cleanup; + goto error; job->client = client; job->msg = msg; @@ -218,21 +217,24 @@ static int virNetServerDispatchNewMessage(virNetServerClientPtr client, } virObjectRef(client); - ret = virThreadPoolSendJob(srv->workers, priority, job); - - if (ret < 0) { + if (virThreadPoolSendJob(srv->workers, priority, job) < 0) { virObjectUnref(client); VIR_FREE(job); virObjectUnref(prog); + goto error; } } else { - ret = virNetServerProcessMsg(srv, client, prog, msg); + if (virNetServerProcessMsg(srv, client, prog, msg) < 0) + goto error; } - cleanup: virObjectUnlock(srv); + return; - return ret; + error: + virNetMessageFree(msg); + virNetServerClientClose(client); + virObjectUnlock(srv); } /** diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 00459d17ba..ea0d5abdee 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -1315,11 +1315,11 @@ static void virNetServerClientDispatchRead(virNetServerClientPtr client) /* Send off to for normal dispatch to workers */ if (msg) { - if (!client->dispatchFunc || - client->dispatchFunc(client, msg, client->dispatchOpaque) < 0) { + if (!client->dispatchFunc) { virNetMessageFree(msg); client->wantClose = true; - return; + } else { + client->dispatchFunc(client, msg, client->dispatchOpaque); } } diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 1a939ad4e1..b21446eeb7 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -36,9 +36,12 @@ typedef virNetServer *virNetServerPtr; typedef struct _virNetServerClient virNetServerClient; typedef virNetServerClient *virNetServerClientPtr; -typedef int (*virNetServerClientDispatchFunc)(virNetServerClientPtr client, - virNetMessagePtr msg, - void *opaque); +/* This function owns the "msg" pointer it is passed and + * must arrange for virNetMessageFree to be called on it + */ +typedef void (*virNetServerClientDispatchFunc)(virNetServerClientPtr client, + virNetMessagePtr msg, + void *opaque); typedef int (*virNetServerClientFilterFunc)(virNetServerClientPtr client, virNetMessagePtr msg,