summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRichard W.M. Jones <rjones@redhat.com>2012-11-23 12:12:33 +0000
committerRichard W.M. Jones <rjones@redhat.com>2012-11-23 12:12:33 +0000
commita6a0b9ba164f68dd79782f054604102017bc9c15 (patch)
treefd19a529e2bbd66c81ee578bde14a323d0b874f3
parentfae8d7cafb66e0da697cde71f7e9444165c76ff7 (diff)
downloadlibguestfs-a6a0b9ba164f68dd79782f054604102017bc9c15.tar.gz
libguestfs-a6a0b9ba164f68dd79782f054604102017bc9c15.tar.xz
libguestfs-a6a0b9ba164f68dd79782f054604102017bc9c15.zip
lib: Fix memory leak and simplify command code.
Fix the following memory leak found by valgrind: ==13629== 498 (112 direct, 386 indirect) bytes in 1 blocks are definitely lost in loss record 99 of 110 ==13629== at 0x4A06B2F: calloc (vg_replace_malloc.c:593) ==13629== by 0x4CA564E: guestfs_safe_calloc (alloc.c:71) ==13629== by 0x4CA9B02: guestfs___new_command (command.c:143) ==13629== by 0x4CA66E9: guestfs___build_appliance (appliance.c:690) ==13629== by 0x4CBD1B9: launch_libvirt (launch-libvirt.c:188) ==13629== by 0x402E7E: main (virt-filesystems.c:349) Also adjust the command code in several places to make it simpler. We can almost always call guestfs___cmd_close right after guestfs___cmd_run, avoiding any need to close the handle along error paths. Tested by running the test suite under valgrind.
-rw-r--r--src/appliance.c7
-rw-r--r--src/fuse.c5
-rw-r--r--src/info.c7
-rw-r--r--src/inspect-icon.c25
-rw-r--r--src/launch-appliance.c7
-rw-r--r--src/launch-libvirt.c3
6 files changed, 16 insertions, 38 deletions
diff --git a/src/appliance.c b/src/appliance.c
index 8a22064b..e7ba60df 100644
--- a/src/appliance.c
+++ b/src/appliance.c
@@ -286,12 +286,10 @@ calculate_supermin_checksum (guestfs_h *g, const char *supermin_path)
guestfs___cmd_set_stdout_callback (cmd, read_checksum, checksum, 0);
r = guestfs___cmd_run (cmd);
+ guestfs___cmd_close (cmd);
/* Errors here are non-fatal, so we don't need to call error(). */
- if (r == -1) {
- guestfs___cmd_close (cmd);
+ if (r == -1)
return NULL;
- }
- guestfs___cmd_close (cmd);
debug (g, "checksum of existing appliance: %s", checksum);
@@ -707,6 +705,7 @@ run_supermin_helper (guestfs_h *g, const char *supermin_path,
guestfs___cmd_add_arg_format (cmd, "%s/root", cachedir);
r = guestfs___cmd_run (cmd);
+ guestfs___cmd_close (cmd);
if (r == -1)
return -1;
if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) {
diff --git a/src/fuse.c b/src/fuse.c
index 76d008a1..ab544f7c 100644
--- a/src/fuse.c
+++ b/src/fuse.c
@@ -1052,7 +1052,7 @@ guestfs__umount_local (guestfs_h *g,
size_t i, tries;
char *localmountpoint;
char *fusermount_log = NULL;
- struct command *cmd = NULL;
+ struct command *cmd;
int r;
FILE *fp;
char error_message[4096];
@@ -1095,7 +1095,6 @@ guestfs__umount_local (guestfs_h *g,
guestfs___cmd_clear_capture_errors (cmd);
r = guestfs___cmd_run (cmd);
guestfs___cmd_close (cmd);
- cmd = NULL;
if (r == -1)
goto out;
if (WIFEXITED (r) && WEXITSTATUS (r) == EXIT_SUCCESS) {
@@ -1127,8 +1126,6 @@ guestfs__umount_local (guestfs_h *g,
}
out:
- if (cmd)
- guestfs___cmd_close (cmd);
if (fusermount_log) {
unlink (fusermount_log);
free (fusermount_log);
diff --git a/src/info.c b/src/info.c
index 156c11fe..6df3aafa 100644
--- a/src/info.c
+++ b/src/info.c
@@ -168,7 +168,7 @@ run_qemu_img_info (guestfs_h *g, const char *filename,
{
char *abs_filename = NULL;
char *safe_filename = NULL;
- struct command *cmd = NULL;
+ struct command *cmd;
int r;
if (guestfs___lazy_make_tmpdir (g) == -1)
@@ -194,6 +194,7 @@ run_qemu_img_info (guestfs_h *g, const char *filename,
guestfs___cmd_add_arg (cmd, safe_filename);
guestfs___cmd_set_stdout_callback (cmd, fn, data, 0);
r = guestfs___cmd_run (cmd);
+ guestfs___cmd_close (cmd);
if (r == -1)
goto error;
if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) {
@@ -201,15 +202,11 @@ run_qemu_img_info (guestfs_h *g, const char *filename,
goto error;
}
- guestfs___cmd_close (cmd);
free (safe_filename);
free (abs_filename);
return 0;
error:
- if (cmd)
- guestfs___cmd_close (cmd);
-
free (safe_filename);
free (abs_filename);
diff --git a/src/inspect-icon.c b/src/inspect-icon.c
index 4831ab1b..4b7f43a2 100644
--- a/src/inspect-icon.c
+++ b/src/inspect-icon.c
@@ -364,7 +364,7 @@ icon_cirros (guestfs_h *g, struct inspect_fs *fs, size_t *size_r)
char *type = NULL;
char *local = NULL;
char *pngfile = NULL;
- struct command *cmd = NULL;
+ struct command *cmd;
int r;
r = guestfs_exists (g, CIRROS_LOGO);
@@ -392,6 +392,7 @@ icon_cirros (guestfs_h *g, struct inspect_fs *fs, size_t *size_r)
guestfs___cmd_add_string_unquoted (cmd, " | " PNMTOPNG " > ");
guestfs___cmd_add_string_quoted (cmd, pngfile);
r = guestfs___cmd_run (cmd);
+ guestfs___cmd_close (cmd);
if (r == -1)
goto out;
if (!WIFEXITED (r) || WEXITSTATUS (r) != 0)
@@ -407,8 +408,6 @@ icon_cirros (guestfs_h *g, struct inspect_fs *fs, size_t *size_r)
free (pngfile);
free (local);
free (type);
- if (cmd)
- guestfs___cmd_close (cmd);
return ret;
}
@@ -438,7 +437,7 @@ icon_windows_xp (guestfs_h *g, struct inspect_fs *fs, const char *explorer,
{
char *ret;
char *pngfile = NULL;
- struct command *cmd = NULL;
+ struct command *cmd;
int r;
pngfile = safe_asprintf (g, "%s/windows-xp-icon.png", g->tmpdir);
@@ -450,14 +449,12 @@ icon_windows_xp (guestfs_h *g, struct inspect_fs *fs, const char *explorer,
" | " BMPTOPNM " | " PNMTOPNG " > ");
guestfs___cmd_add_string_quoted (cmd, pngfile);
r = guestfs___cmd_run (cmd);
+ guestfs___cmd_close (cmd);
if (r == -1)
goto error;
if (!WIFEXITED (r) || WEXITSTATUS (r) != 0)
goto not_found;
- guestfs___cmd_close (cmd);
- cmd = NULL;
-
if (read_whole_file (g, pngfile, &ret, size_r) == -1)
goto error;
@@ -466,14 +463,10 @@ icon_windows_xp (guestfs_h *g, struct inspect_fs *fs, const char *explorer,
return ret;
error:
- if (cmd)
- guestfs___cmd_close (cmd);
free (pngfile);
return NULL;
not_found:
- if (cmd)
- guestfs___cmd_close (cmd);
free (pngfile);
return NOT_FOUND;
}
@@ -484,7 +477,7 @@ icon_windows_7 (guestfs_h *g, struct inspect_fs *fs, const char *explorer,
{
char *ret;
char *pngfile = NULL;
- struct command *cmd = NULL;
+ struct command *cmd;
int r;
pngfile = safe_asprintf (g, "%s/windows-7-icon.png", g->tmpdir);
@@ -498,14 +491,12 @@ icon_windows_7 (guestfs_h *g, struct inspect_fs *fs, const char *explorer,
PNMTOPNG " > ");
guestfs___cmd_add_string_quoted (cmd, pngfile);
r = guestfs___cmd_run (cmd);
+ guestfs___cmd_close (cmd);
if (r == -1)
goto error;
if (!WIFEXITED (r) || WEXITSTATUS (r) != 0)
goto not_found;
- guestfs___cmd_close (cmd);
- cmd = NULL;
-
if (read_whole_file (g, pngfile, &ret, size_r) == -1)
goto error;
@@ -514,14 +505,10 @@ icon_windows_7 (guestfs_h *g, struct inspect_fs *fs, const char *explorer,
return ret;
error:
- if (cmd)
- guestfs___cmd_close (cmd);
free (pngfile);
return NULL;
not_found:
- if (cmd)
- guestfs___cmd_close (cmd);
free (pngfile);
return NOT_FOUND;
}
diff --git a/src/launch-appliance.c b/src/launch-appliance.c
index 17e90d9e..8b2fda07 100644
--- a/src/launch-appliance.c
+++ b/src/launch-appliance.c
@@ -755,9 +755,9 @@ test_qemu (guestfs_h *g)
guestfs___cmd_set_stdout_callback (cmd, read_all, &g->app.qemu_help,
CMD_STDOUT_FLAG_WHOLE_BUFFER);
r = guestfs___cmd_run (cmd);
+ guestfs___cmd_close (cmd);
if (r == -1 || !WIFEXITED (r) || WEXITSTATUS (r) != 0)
goto error;
- guestfs___cmd_close (cmd);
cmd = guestfs___new_command (g);
guestfs___cmd_add_arg (cmd, g->qemu);
@@ -766,9 +766,9 @@ test_qemu (guestfs_h *g)
guestfs___cmd_set_stdout_callback (cmd, read_all, &g->app.qemu_version,
CMD_STDOUT_FLAG_WHOLE_BUFFER);
r = guestfs___cmd_run (cmd);
+ guestfs___cmd_close (cmd);
if (r == -1 || !WIFEXITED (r) || WEXITSTATUS (r) != 0)
goto error;
- guestfs___cmd_close (cmd);
cmd = guestfs___new_command (g);
guestfs___cmd_add_arg (cmd, g->qemu);
@@ -782,15 +782,14 @@ test_qemu (guestfs_h *g)
guestfs___cmd_set_stdout_callback (cmd, read_all, &g->app.qemu_devices,
CMD_STDOUT_FLAG_WHOLE_BUFFER);
r = guestfs___cmd_run (cmd);
+ guestfs___cmd_close (cmd);
if (r == -1 || !WIFEXITED (r) || WEXITSTATUS (r) != 0)
goto error;
- guestfs___cmd_close (cmd);
return 0;
error:
error (g, _("qemu command failed\nIf qemu is located on a non-standard path, try setting the LIBGUESTFS_QEMU\nenvironment variable. There may also be errors printed above."));
- guestfs___cmd_close (cmd);
return -1;
}
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index e4df2a6b..3ee4c0f8 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -1190,18 +1190,17 @@ make_qcow2_overlay (guestfs_h *g, const char *path, const char *format)
}
guestfs___cmd_add_arg (cmd, tmpfile);
r = guestfs___cmd_run (cmd);
+ guestfs___cmd_close (cmd);
if (r == -1)
goto error;
if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) {
error (g, _("qemu-img create: could not create snapshot over %s"), path);
goto error;
}
- guestfs___cmd_close (cmd);
return tmpfile; /* caller frees */
error:
- guestfs___cmd_close (cmd);
free (tmpfile);
return NULL;