From a6a0b9ba164f68dd79782f054604102017bc9c15 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Fri, 23 Nov 2012 12:12:33 +0000 Subject: 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. --- src/appliance.c | 7 +++---- src/fuse.c | 5 +---- src/info.c | 7 ++----- src/inspect-icon.c | 25 ++++++------------------- src/launch-appliance.c | 7 +++---- src/launch-libvirt.c | 3 +-- 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; -- cgit