[PATCH 1/2] examples: code cleanups
do a code cleanup by removing the tailing spaces Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> --- examples/create_disk.c | 2 +- examples/inspect_vm.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/create_disk.c b/examples/create_disk.c index f4b9044..bcad6d8 100644 --- a/examples/create_disk.c +++ b/examples/create_disk.c @@ -7,7 +7,7 @@ #include <unistd.h> #include <guestfs.h> -int +int main (int argc, char *argv[]) { guestfs_h *g; diff --git a/examples/inspect_vm.c b/examples/inspect_vm.c index 6c45aac..5a61904 100644 --- a/examples/inspect_vm.c +++ b/examples/inspect_vm.c @@ -21,7 +21,7 @@ count_strings (char *const *argv) return c; } -int +int main (int argc, char *argv[]) { guestfs_h *g; -- 1.7.9
The strdup/strndup() introduces malloc() to allocate memory, so we need to free them carefully. Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> --- format/format.c | 88 +++++++++++++++++++++++++++++++++--------------------- 1 files changed, 54 insertions(+), 34 deletions(-) diff --git a/format/format.c b/format/format.c index b3a47cd..cc7563d 100644 --- a/format/format.c +++ b/format/format.c @@ -51,7 +51,7 @@ static const char *vg = NULL, *lv = NULL; static const char *partition = "DEFAULT"; static int wipe = 0; -static void parse_vg_lv (const char *lvm); +static int parse_vg_lv (const char *lvm); static int do_format (void); static int do_rescan (char **devices); @@ -61,7 +61,7 @@ bad_cast (char const *s) return (char *) s; } -static void __attribute__((noreturn)) +static void usage (int status) { char *warning = @@ -96,7 +96,6 @@ usage (int status) program_name, warning, program_name, program_name, warning); } - exit (status); } int @@ -156,7 +155,7 @@ main (int argc, char *argv[]) else if (optarg[0] == '-') { /* eg: --filesystem --lvm */ fprintf (stderr, _("%s: no filesystem was specified\n"), program_name); - exit (EXIT_FAILURE); + goto fail; } else filesystem = optarg; } else if (STREQ (long_options[option_index].name, "lvm")) { @@ -164,17 +163,19 @@ main (int argc, char *argv[]) fprintf (stderr, _("%s: --lvm option cannot be given multiple times\n"), program_name); - exit (EXIT_FAILURE); + goto fail; } if (optarg == NULL) { vg = strdup ("VG"); lv = strdup ("LV"); - if (!vg || !lv) { perror ("strdup"); exit (EXIT_FAILURE); } + if (!vg || !lv) { perror ("strdup"); goto fail; } } else if (STREQ (optarg, "none")) vg = lv = NULL; - else - parse_vg_lv (optarg); + else { + if (parse_vg_lv (optarg) == -1) + goto fail; + } } else if (STREQ (long_options[option_index].name, "partition")) { if (optarg == NULL) partition = "DEFAULT"; @@ -187,7 +188,7 @@ main (int argc, char *argv[]) } else { fprintf (stderr, _("%s: unknown long option: %s (%d)\n"), program_name, long_options[option_index].name, option_index); - exit (EXIT_FAILURE); + goto fail; } break; @@ -209,9 +210,11 @@ main (int argc, char *argv[]) case HELP_OPTION: usage (EXIT_SUCCESS); + exit (EXIT_SUCCESS); default: usage (EXIT_FAILURE); + goto fail; } } @@ -224,12 +227,16 @@ main (int argc, char *argv[]) assert (live == 0); /* Must be no extra arguments on the command line. */ - if (optind != argc) - usage (EXIT_FAILURE); + if (optind != argc) { + usage(EXIT_FAILURE); + goto fail; + } /* The user didn't specify any drives to format. */ - if (drvs == NULL) + if (drvs == NULL) { usage (EXIT_FAILURE); + goto fail; + } /* Because the libguestfs kernel can get stuck rereading the * partition table after things have been erased, we sometimes need @@ -240,11 +247,13 @@ main (int argc, char *argv[]) add_drives (drvs, 'a'); if (guestfs_launch (g) == -1) - exit (EXIT_FAILURE); + goto fail; /* Perform the format. */ retry = do_format (); - if (!retry) + if (retry == -1) + goto fail; + else if (!retry) break; if (retries == 0) { @@ -266,22 +275,31 @@ main (int argc, char *argv[]) "data which we are unable to remove. If you think this\n" "is a bug, please file a bug at http://libguestfs.org/\n"), program_name); - exit (EXIT_FAILURE); + goto fail; } } + free ((char *) vg); + free ((char *) lv); /* Free up data structures. */ free_drives (drvs); guestfs_close (g); exit (EXIT_SUCCESS); + +fail: + free ((char *)vg); + free ((char *)lv); + free_drives (drvs); + guestfs_close (g); + exit (EXIT_FAILURE); } /* Parse lvm string of the form "/dev/VG/LV" or "VG/LV". * This sets the global variables 'vg' and 'lv', or exits on failure. */ -static void +static int parse_vg_lv (const char *lvm) { size_t i; @@ -296,20 +314,22 @@ parse_vg_lv (const char *lvm) if (!vg || !lv) { perror ("strdup"); - exit (EXIT_FAILURE); + return -1; } } else { cannot_parse: fprintf (stderr, _("%s: cannot parse --lvm option (%s)\n"), program_name, lvm); - exit (EXIT_FAILURE); + return -1; } if (strchr (vg, '/') || strchr (lv, '/')) goto cannot_parse; + + return 0; } -/* Returns 0 on success, 1 if we need to retry. */ +/* Returns 0 on success, 1 if we need to retry. -1 for fail */ static int do_format (void) { @@ -319,7 +339,7 @@ do_format (void) devices = guestfs_list_devices (g); if (devices == NULL) - exit (EXIT_FAILURE); + return -1; /* Erase the disks. */ if (!wipe) { @@ -328,11 +348,11 @@ do_format (void) /* No wipe, but get rid of LVM metadata by erasing each partition. */ parts = guestfs_list_partitions (g); if (parts == NULL) - exit (EXIT_FAILURE); + return -1; for (i = 0; parts[i] != NULL; ++i) { if (guestfs_zero (g, parts[i]) == -1) - exit (EXIT_FAILURE); + return -1; free (parts[i]); } free (parts); @@ -340,13 +360,13 @@ do_format (void) /* Then erase the partition table on each device. */ for (i = 0; devices[i] != NULL; ++i) { if (guestfs_zero (g, devices[i]) == -1) - exit (EXIT_FAILURE); + return -1; } } else /* wipe */ { for (i = 0; devices[i] != NULL; ++i) { if (guestfs_zero_device (g, devices[i]) == -1) - exit (EXIT_FAILURE); + return -1; } } @@ -368,15 +388,15 @@ do_format (void) if (STREQ (partition, "DEFAULT")) { dev_size = guestfs_blockdev_getsize64 (g, devices[i]); if (dev_size == -1) - exit (EXIT_FAILURE); + return -1; ptype = dev_size < INT64_C(2)*1024*1024*1024*1024 ? "mbr" : "gpt"; } if (guestfs_part_disk (g, devices[i], ptype) == -1) - exit (EXIT_FAILURE); + return -1; if (asprintf (&dev, "%s1", devices[i]) == -1) { perror ("asprintf"); - exit (EXIT_FAILURE); + return -1; } free_dev = 1; } @@ -385,29 +405,29 @@ do_format (void) char *devs[2] = { dev, NULL }; if (guestfs_pvcreate (g, dev) == -1) - exit (EXIT_FAILURE); + return -1; if (guestfs_vgcreate (g, vg, devs) == -1) - exit (EXIT_FAILURE); + return -1; if (guestfs_lvcreate (g, lv, vg, 32) == -1) /* 32 MB is smallest LV */ - exit (EXIT_FAILURE); + return -1; if (free_dev) free (dev); if (asprintf (&dev, "/dev/%s/%s", vg, lv) == -1) { perror ("asprintf"); - exit (EXIT_FAILURE); + return -1; } free_dev = 1; if (guestfs_lvresize_free (g, dev, 100) == -1) - exit (EXIT_FAILURE); + return -1; } if (filesystem) { if (guestfs_mkfs_opts (g, filesystem, dev, -1) == -1) - exit (EXIT_FAILURE); + return -1; } if (free_dev) @@ -415,7 +435,7 @@ do_format (void) } if (guestfs_sync (g) == -1) - exit (EXIT_FAILURE); + return -1; ret = 0; -- 1.7.9
On Tue, Jan 31, 2012 at 06:19:32PM +0800, Wanlong Gao wrote:
The strdup/strndup() introduces malloc() to allocate memory, so we need to free them carefully.
I'm not sure this patch is necessary. We already check for memory leaks in many utilities (in 'make extra-tests'), but we only do it for the success case. If the utility fails, it's better that it just exits as soon as possible, even if it doesn't free all memory along the way. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
On 01/31/2012 09:30 PM, Richard W.M. Jones wrote:
On Tue, Jan 31, 2012 at 06:19:32PM +0800, Wanlong Gao wrote:
The strdup/strndup() introduces malloc() to allocate memory, so we need to free them carefully.
I'm not sure this patch is necessary.
We already check for memory leaks in many utilities (in 'make extra-tests'), but we only do it for the success case. If the utility fails, it's better that it just exits as soon as possible, even if it doesn't free all memory along the way.
But I wonder that if fail, where will the allocated memory go? not a leak? And, when successfully exit, the implied allocated memory is also not freed here, either. make extra-tests can test virt-format, too, it seems not? Thanks -Wanlong Gao
Rich.
On Tue, Jan 31, 2012 at 09:39:17PM +0800, Wanlong Gao wrote:
On 01/31/2012 09:30 PM, Richard W.M. Jones wrote:
On Tue, Jan 31, 2012 at 06:19:32PM +0800, Wanlong Gao wrote:
The strdup/strndup() introduces malloc() to allocate memory, so we need to free them carefully.
I'm not sure this patch is necessary.
We already check for memory leaks in many utilities (in 'make extra-tests'), but we only do it for the success case. If the utility fails, it's better that it just exits as soon as possible, even if it doesn't free all memory along the way.
But I wonder that if fail, where will the allocated memory go? not a leak?
The operating system recovers memory from all processes that call _exit(2). There is no need to free any of it. But we do want to know if the library is leaking memory, which would be a problem for people using libguestfs, which is why we perform leak tests in the non-failure case. [...]
make extra-tests can test virt-format, too, it seems not?
It's tested: Makefile.am: $(RUN_VG) ../../format/virt-format -a test1.img >/dev/null Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones New in Fedora 11: Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 70 libraries supprt'd http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
On 01/31/2012 09:53 PM, Richard W.M. Jones wrote:
On Tue, Jan 31, 2012 at 09:39:17PM +0800, Wanlong Gao wrote:
On 01/31/2012 09:30 PM, Richard W.M. Jones wrote:
On Tue, Jan 31, 2012 at 06:19:32PM +0800, Wanlong Gao wrote:
The strdup/strndup() introduces malloc() to allocate memory, so we need to free them carefully.
I'm not sure this patch is necessary.
We already check for memory leaks in many utilities (in 'make extra-tests'), but we only do it for the success case. If the utility fails, it's better that it just exits as soon as possible, even if it doesn't free all memory along the way.
But I wonder that if fail, where will the allocated memory go? not a leak?
The operating system recovers memory from all processes that call _exit(2). There is no need to free any of it.
But we do want to know if the library is leaking memory, which would be a problem for people using libguestfs, which is why we perform leak tests in the non-failure case.
Yes, I see, only shared memory and temp files exist after the process' lifetime. thanks for you explanation Rich. -Wanlong Gao
[...]
make extra-tests can test virt-format, too, it seems not?
It's tested:
Makefile.am: $(RUN_VG) ../../format/virt-format -a test1.img >/dev/null
Rich.
On Tue, Jan 31, 2012 at 06:19:31PM +0800, Wanlong Gao wrote:
do a code cleanup by removing the tailing spaces
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> --- examples/create_disk.c | 2 +- examples/inspect_vm.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/examples/create_disk.c b/examples/create_disk.c index f4b9044..bcad6d8 100644 --- a/examples/create_disk.c +++ b/examples/create_disk.c @@ -7,7 +7,7 @@ #include <unistd.h> #include <guestfs.h>
-int +int main (int argc, char *argv[]) { guestfs_h *g; diff --git a/examples/inspect_vm.c b/examples/inspect_vm.c index 6c45aac..5a61904 100644 --- a/examples/inspect_vm.c +++ b/examples/inspect_vm.c @@ -21,7 +21,7 @@ count_strings (char *const *argv) return c; }
-int +int main (int argc, char *argv[]) { guestfs_h *g; -- 1.7.9
Thanks, I'll push this. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
participants (3)
-
Richard W.M. Jones -
Wanlong Gao -
Wanlong Gao