[libvirt] [PATCH] Fix memory leak in virCommandRun()

--- src/util/command.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index c0520ec..473d1fc 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -956,6 +956,8 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) cmd->errbuf = NULL; } + VIR_FREE(outbuf); + VIR_FREE(errbuf); return ret; } -- 1.7.3

On 12/07/2010 11:10 PM, Hu Tao wrote:
--- src/util/command.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/util/command.c b/src/util/command.c index c0520ec..473d1fc 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -956,6 +956,8 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) cmd->errbuf = NULL; }
+ VIR_FREE(outbuf); + VIR_FREE(errbuf);
Good catch, but not quite right. I inadvertently disabled logging abilities with commit ee11729d7, and in so doing, outbuf and errbuf were never populated because virCommandProcessIO was skipped. Here's a better patch: From 9ce936ee395e967551e36598ff0650315ac7686e Mon Sep 17 00:00:00 2001 From: Eric Blake <eblake@redhat.com> Date: Wed, 8 Dec 2010 08:03:29 -0700 Subject: [PATCH] command: avoid memory leak * src/util/command.c (virCommandRun): Fix yesterday's regression on logging, and avoid leaking log-only output captures. Reported by Hu Tao. --- src/util/command.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index e39e949..089e0bd 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -964,10 +964,12 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) if (!cmd->outfdptr) { cmd->outfdptr = &cmd->outfd; cmd->outbuf = &outbuf; + string_io = true; } if (!cmd->errfdptr) { cmd->errfdptr = &cmd->errfd; cmd->errbuf = &errbuf; + string_io = true; } if (virCommandRunAsync(cmd, NULL) < 0) { @@ -1009,6 +1011,7 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) VIR_DEBUG("ignoring failed close on fd %d", tmpfd); cmd->outfdptr = NULL; cmd->outbuf = NULL; + VIR_FREE(outbuf); } if (cmd->errbuf == &errbuf) { int tmpfd = cmd->errfd; @@ -1016,6 +1019,7 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) VIR_DEBUG("ignoring failed close on fd %d", tmpfd); cmd->errfdptr = NULL; cmd->errbuf = NULL; + VIR_FREE(errbuf); } return ret; -- 1.7.3.2 -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Wed, Dec 08, 2010 at 08:04:28AM -0700, Eric Blake wrote:
On 12/07/2010 11:10 PM, Hu Tao wrote:
--- src/util/command.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/util/command.c b/src/util/command.c index c0520ec..473d1fc 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -956,6 +956,8 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) cmd->errbuf = NULL; }
+ VIR_FREE(outbuf); + VIR_FREE(errbuf);
Good catch, but not quite right. I inadvertently disabled logging abilities with commit ee11729d7, and in so doing, outbuf and errbuf were never populated because virCommandProcessIO was skipped. Here's a better patch:
From 9ce936ee395e967551e36598ff0650315ac7686e Mon Sep 17 00:00:00 2001 From: Eric Blake <eblake@redhat.com> Date: Wed, 8 Dec 2010 08:03:29 -0700 Subject: [PATCH] command: avoid memory leak
* src/util/command.c (virCommandRun): Fix yesterday's regression on logging, and avoid leaking log-only output captures. Reported by Hu Tao. --- src/util/command.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/util/command.c b/src/util/command.c index e39e949..089e0bd 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -964,10 +964,12 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) if (!cmd->outfdptr) { cmd->outfdptr = &cmd->outfd; cmd->outbuf = &outbuf; + string_io = true; } if (!cmd->errfdptr) { cmd->errfdptr = &cmd->errfd; cmd->errbuf = &errbuf; + string_io = true; }
if (virCommandRunAsync(cmd, NULL) < 0) { @@ -1009,6 +1011,7 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) VIR_DEBUG("ignoring failed close on fd %d", tmpfd); cmd->outfdptr = NULL; cmd->outbuf = NULL; + VIR_FREE(outbuf); } if (cmd->errbuf == &errbuf) { int tmpfd = cmd->errfd; @@ -1016,6 +1019,7 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) VIR_DEBUG("ignoring failed close on fd %d", tmpfd); cmd->errfdptr = NULL; cmd->errbuf = NULL; + VIR_FREE(errbuf); }
return ret;
ACK. -- Thanks, Hu Tao

On 12/08/2010 05:33 PM, Hu Tao wrote:
From 9ce936ee395e967551e36598ff0650315ac7686e Mon Sep 17 00:00:00 2001 From: Eric Blake <eblake@redhat.com> Date: Wed, 8 Dec 2010 08:03:29 -0700 Subject: [PATCH] command: avoid memory leak
* src/util/command.c (virCommandRun): Fix yesterday's regression on logging, and avoid leaking log-only output captures. Reported by Hu Tao.
ACK.
Thanks; pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Hu Tao