[PATCH 1/2] virnetsocket: Don't free virCommand in virNetSocketNewConnectCommand()

[Ján Tomko]

On a Friday in 2021, Michal Privoznik wrote:
>The aim of virNetSocketNewConnectCommand() is to execute passed
>command and attach socket pair/pipe to it so that client socket
>can be opened (this is used for connections with alternative
>transports, e.g. ssh). The virCommand is created in a caller and
>then passed to virNetSocketNewConnectCommand() where it is freed
>using virCommandFree(). This approach is wrong on two levels:
>
>1) The deallocation happens on a different level than allocation,
>2) There's a WIN32 stub that just reports an error and doesn't
>   free the command.
>
>However, with g_autoptr() trickery the command can be freed in
>caller.
>
>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
> src/rpc/virnetsocket.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
>diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
>index 212089520d..50c0c4ecc8 100644
>--- a/src/rpc/virnetsocket.c
>+++ b/src/rpc/virnetsocket.c
>@@ -821,8 +821,6 @@ int virNetSocketNewConnectCommand(virCommand *cmd,
>     if (!(*retsock = virNetSocketNew(NULL, NULL, true, sv[0], errfd[0], pid, false)))
>         goto error;
>
>-    virCommandFree(cmd);
>-
>     return 0;
>
>  error:
>@@ -832,7 +830,6 @@ int virNetSocketNewConnectCommand(virCommand *cmd,
>     VIR_FORCE_CLOSE(errfd[1]);
>
>     virCommandAbort(cmd);
>-    virCommandFree(cmd);
>
>     return -1;
> }


The function is also called in tests/virnetsockettest.c

If you free the command there as well:

Reviewed-by: Ján Tomko <jtomko at redhat.com>

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20211015/42e8acbc/attachment-0001.sig>