On 9/30/22 09:02, Jiang Jiacheng wrote:
> From: jiangjiacheng <jiangjiacheng@xxxxxxxxxx>
>
> In virNetServerProgramDispatchCall, The arg is passed as a void* and used to point
> to a certain struct depended on the dispatcher, so I think it's the memory of the
> struct's member that leaks and this memory shuld be freed by xdr_free.
>
> In virNetServerClientNew, client->rx is assigned by invoking virNetServerClientNew,
> but isn't freed if client->privateData's initialization failed, which leads to a
> memory leak. Thanks to Liang Peng's suggestion, put virNetMessageFree(client->rx)
> into virNetServerClientDispose() to release the memory.
>
> diff to v2:
> - virNetServerProgramDispatchCall, only free the memory in successful path and error
> label.
This ^^^^
>
> Signed-off-by: jiangjiacheng <jiangjiacheng@xxxxxxxxxx>
> ---
usually goes here.
> src/rpc/virnetserverclient.c | 2 ++
> src/rpc/virnetserverprogram.c | 22 ++++++++++------------
> 2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
> index a7d2dfa795..30f6af7be5 100644
> --- a/src/rpc/virnetserverclient.c
> +++ b/src/rpc/virnetserverclient.c
> @@ -931,6 +931,8 @@ void virNetServerClientDispose(void *obj)
> PROBE(RPC_SERVER_CLIENT_DISPOSE,
> "client=%p", client);
>
> + if (client->rx)
> + virNetMessageFree(client->rx);
> if (client->privateData)
> client->privateDataFreeFunc(client->privateData);
>
> diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c
> index 3ddf9f0428..94660f867a 100644
> --- a/src/rpc/virnetserverprogram.c
> +++ b/src/rpc/virnetserverprogram.c
> @@ -368,7 +368,7 @@ virNetServerProgramDispatchCall(virNetServerProgram *prog,
> g_autofree char *arg = NULL;
> g_autofree char *ret = NULL;
> int rv = -1;
> - virNetServerProgramProc *dispatcher;
> + virNetServerProgramProc *dispatcher = NULL;
> virNetMessageError rerr;
> size_t i;
> g_autoptr(virIdentity) identity = NULL;
> @@ -446,8 +446,6 @@ virNetServerProgramDispatchCall(virNetServerProgram *prog,
> msg->nfds = 0;
> }
>
> - xdr_free(dispatcher->arg_filter, arg);
> -
> if (rv < 0)
> goto error;
>
> @@ -460,28 +458,28 @@ virNetServerProgramDispatchCall(virNetServerProgram *prog,
> /*msg->header.serial = msg->header.serial;*/
> msg->header.status = VIR_NET_OK;
>
> - if (virNetMessageEncodeHeader(msg) < 0) {
> - xdr_free(dispatcher->ret_filter, ret);
> + if (virNetMessageEncodeHeader(msg) < 0)
> goto error;
> - }
>
> if (msg->nfds &&
> - virNetMessageEncodeNumFDs(msg) < 0) {
> - xdr_free(dispatcher->ret_filter, ret);
> + virNetMessageEncodeNumFDs(msg) < 0)
> goto error;
> - }
>
> - if (virNetMessageEncodePayload(msg, dispatcher->ret_filter, ret) < 0) {
> - xdr_free(dispatcher->ret_filter, ret);
> + if (virNetMessageEncodePayload(msg, dispatcher->ret_filter, ret) < 0)
> goto error;
> - }
>
> + xdr_free(dispatcher->arg_filter, arg);
> xdr_free(dispatcher->ret_filter, ret);
>
> /* Put reply on end of tx queue to send out */
> return virNetServerClientSendMessage(client, msg);
>
> error:
> + if (dispatcher) {
> + xdr_free(dispatcher->arg_filter, arg);
> + xdr_free(dispatcher->ret_filter, ret);
> + }
> +
This adds needless whitespace.
Reviewed-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
and pushed.
Michal
