[libvirt] [PATCH 2/5] rpc: simplify calling convention of virNetServerClientDispatchFunc
Jim Fehlig
jfehlig at suse.com
Tue Mar 6 23:06:57 UTC 2018
On 03/06/2018 10:58 AM, Daniel P. Berrangé wrote:
> 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 at redhat.com>
> ---
> src/rpc/virnetserver.c | 24 +++++++++++++-----------
> src/rpc/virnetserverclient.c | 6 +++---
> src/rpc/virnetserverclient.h | 9 ++++++---
> 3 files changed, 22 insertions(+), 17 deletions(-)
Same as 1/1. Looks like a nice improvement regardless of lost locks.
Reviewed-by: Jim Fehlig <jfehlig at suse.com>
Regards,
Jim
>
> 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,
>
More information about the libvir-list
mailing list