| Message ID | 1455623393-17364-1-git-send-email-ian.campbell@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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>
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>
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; }
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(-)