All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libxl: close fd's in parent when spawning qdisk
@ 2016-02-16 11:49 Ian Campbell
  2016-02-16 12:59 ` Wei Liu
  0 siblings, 1 reply; 3+ messages in thread
From: Ian Campbell @ 2016-02-16 11:49 UTC (permalink / raw)
  To: ian.jackson, wei.liu2, xen-devel; +Cc: Ian Campbell

Coverity points out that these remain open in the parent upon
success, which is a resource leak.

To fix this rejig the exit paths such that success and error cases
both close the two fds, this means adjusting the callback to only
happen for the error case and it also makes sense to rename the label
from "error" to just "out".

Compile tested only.

CID: 1130518 (null) and 1130517 (logfile_w).

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_dm.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index a088d71..4aca38e 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1999,12 +1999,12 @@ void libxl__spawn_qdisk_backend(libxl__egc *egc, libxl__dm_spawn_state *dmss)
     logfile_w = libxl__create_qemu_logfile(gc, GCSPRINTF("qdisk-%u", domid));
     if (logfile_w < 0) {
         rc = logfile_w;
-        goto error;
+        goto out;
     }
     null = open("/dev/null", O_RDONLY);
     if (null < 0) {
        rc = ERROR_FAIL;
-       goto error;
+       goto out;
     }
 
     dmss->guest_config = NULL;
@@ -2030,19 +2030,18 @@ void libxl__spawn_qdisk_backend(libxl__egc *egc, libxl__dm_spawn_state *dmss)
     dmss->spawn.detached_cb = device_model_detached;
     rc = libxl__spawn_spawn(egc, &dmss->spawn);
     if (rc < 0)
-        goto error;
+        goto out;
     if (!rc) { /* inner child */
         setsid();
         libxl__exec(gc, null, logfile_w, logfile_w, dm, args, NULL);
     }
 
-    return;
-
-error:
-    assert(rc);
+    rc = 0;
+out:
     if (logfile_w >= 0) close(logfile_w);
     if (null >= 0) close(null);
-    dmss->callback(egc, dmss, rc);
+    /* callback on error only, success goes via dmss->spawn.*_cb */
+    if (rc) dmss->callback(egc, dmss, rc);
     return;
 }
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] libxl: close fd's in parent when spawning qdisk
  2016-02-16 11:49 [PATCH] libxl: close fd's in parent when spawning qdisk Ian Campbell
@ 2016-02-16 12:59 ` Wei Liu
  2016-02-16 17:43   ` Ian Jackson
  0 siblings, 1 reply; 3+ messages in thread
From: Wei Liu @ 2016-02-16 12:59 UTC (permalink / raw)
  To: Ian Campbell; +Cc: wei.liu2, ian.jackson, xen-devel

On Tue, Feb 16, 2016 at 11:49:53AM +0000, Ian Campbell wrote:
> Coverity points out that these remain open in the parent upon
> success, which is a resource leak.
> 
> To fix this rejig the exit paths such that success and error cases
> both close the two fds, this means adjusting the callback to only
> happen for the error case and it also makes sense to rename the label
> from "error" to just "out".
> 
> Compile tested only.
> 
> CID: 1130518 (null) and 1130517 (logfile_w).
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] libxl: close fd's in parent when spawning qdisk
  2016-02-16 12:59 ` Wei Liu
@ 2016-02-16 17:43   ` Ian Jackson
  0 siblings, 0 replies; 3+ messages in thread
From: Ian Jackson @ 2016-02-16 17:43 UTC (permalink / raw)
  To: Wei Liu; +Cc: Ian Campbell, xen-devel

Wei Liu writes ("Re: [PATCH] libxl: close fd's in parent when spawning qdisk"):
> On Tue, Feb 16, 2016 at 11:49:53AM +0000, Ian Campbell wrote:
> > Coverity points out that these remain open in the parent upon
> > success, which is a resource leak.
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-02-16 17:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-16 11:49 [PATCH] libxl: close fd's in parent when spawning qdisk Ian Campbell
2016-02-16 12:59 ` Wei Liu
2016-02-16 17:43   ` Ian Jackson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.