From b1919066ca3d83f11b72d38d64f25739bd0ff67e Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Thu, 22 Aug 2013 19:48:05 +0100 Subject: [PATCH] Initialize CLEANUP_* stack variables with NULL in various places. Code like: CLEANUP_FREE char *buf; /* some code which might return early */ buf = malloc (10); is a potential bug because the free (*buf) might be called when buf is an uninitialized pointer. Initialize buf = NULL to avoid this. Several of these are bugs, most are not bugs (because there is no early return statement before the variable gets initialized). However the compiler can elide the initialization, and even if it does not the performance "penalty" is miniscule, and correctness is better. --- cat/virt-filesystems.c | 2 +- daemon/augeas.c | 2 +- daemon/debug.c | 6 +++--- daemon/ext2.c | 4 ++-- daemon/file.c | 2 +- daemon/guestfsd.c | 4 ++-- daemon/initrd.c | 2 +- daemon/ls.c | 2 +- daemon/xattr.c | 2 +- fish/copy.c | 2 +- fish/inspect.c | 2 +- src/inspect-fs-unix.c | 2 +- src/inspect-fs-windows.c | 2 +- src/launch-libvirt.c | 2 +- 14 files changed, 18 insertions(+), 18 deletions(-) diff --git a/cat/virt-filesystems.c b/cat/virt-filesystems.c index 6e86bfc54..434a7b434 100644 --- a/cat/virt-filesystems.c +++ b/cat/virt-filesystems.c @@ -688,7 +688,7 @@ do_output_blockdevs (void) for (i = 0; devices[i] != NULL; ++i) { int64_t size = -1; CLEANUP_FREE_STRING_LIST char **parents = NULL; - CLEANUP_FREE char *dev; + CLEANUP_FREE char *dev = NULL; dev = guestfs_canonical_device_name (g, devices[i]); if (!dev) diff --git a/daemon/augeas.c b/daemon/augeas.c index d3fe7db62..17d5c299f 100644 --- a/daemon/augeas.c +++ b/daemon/augeas.c @@ -355,7 +355,7 @@ do_aug_ls (const char *path) if (STREQ (path, "/")) matches = do_aug_match ("/*"); else { - CLEANUP_FREE char *buf; + CLEANUP_FREE char *buf = NULL; len += 3; /* / * + terminating \0 */ buf = malloc (len); diff --git a/daemon/debug.c b/daemon/debug.c index be2a6b8cd..dca6edb8d 100644 --- a/daemon/debug.c +++ b/daemon/debug.c @@ -325,7 +325,7 @@ debug_binaries (const char *subcmd, size_t argc, char *const *const argv) { int r; char *out; - CLEANUP_FREE char *err; + CLEANUP_FREE char *err = NULL; char cmd[256]; snprintf (cmd, sizeof (cmd), @@ -395,7 +395,7 @@ debug_ls (const char *subcmd, size_t argc, char *const *const argv) size_t i; int r; char *out; - CLEANUP_FREE char *err; + CLEANUP_FREE char *err = NULL; cargv[0] = str_ls; cargv[1] = "-a"; @@ -422,7 +422,7 @@ debug_ll (const char *subcmd, size_t argc, char *const *const argv) size_t i; int r; char *out; - CLEANUP_FREE char *err; + CLEANUP_FREE char *err = NULL; cargv[0] = str_ls; cargv[1] = "-la"; diff --git a/daemon/ext2.c b/daemon/ext2.c index dac8e0094..a49014f49 100644 --- a/daemon/ext2.c +++ b/daemon/ext2.c @@ -649,9 +649,9 @@ char * do_get_e2attrs (const char *filename) { int r; - CLEANUP_FREE char *buf; + CLEANUP_FREE char *buf = NULL; char *out; - CLEANUP_FREE char *err; + CLEANUP_FREE char *err = NULL; size_t i, j; buf = sysroot_path (filename); diff --git a/daemon/file.c b/daemon/file.c index 9c29ec3bc..7f39e622a 100644 --- a/daemon/file.c +++ b/daemon/file.c @@ -492,7 +492,7 @@ do_file (const char *path) const char *flags = is_dev ? "-zbsL" : "-zb"; char *out; - CLEANUP_FREE char *err; + CLEANUP_FREE char *err = NULL; int r = command (&out, &err, str_file, flags, path, NULL); if (r == -1) { diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c index b55f88a58..195fda011 100644 --- a/daemon/guestfsd.c +++ b/daemon/guestfsd.c @@ -710,7 +710,7 @@ commandf (char **stdoutput, char **stderror, int flags, const char *name, ...) { va_list args; /* NB: Mustn't free the strings which are on the stack. */ - CLEANUP_FREE const char **argv; + CLEANUP_FREE const char **argv = NULL; char *s; size_t i; int r; @@ -754,7 +754,7 @@ int commandrf (char **stdoutput, char **stderror, int flags, const char *name, ...) { va_list args; - CLEANUP_FREE const char **argv; + CLEANUP_FREE const char **argv = NULL; char *s; int i, r; diff --git a/daemon/initrd.c b/daemon/initrd.c index e26d98468..ea34000d4 100644 --- a/daemon/initrd.c +++ b/daemon/initrd.c @@ -86,7 +86,7 @@ char * do_initrd_cat (const char *path, const char *filename, size_t *size_r) { char tmpdir[] = "/tmp/initrd-cat-XXXXXX"; - CLEANUP_FREE char *cmd; + CLEANUP_FREE char *cmd = NULL; struct stat statbuf; int fd, r; char *ret = NULL; diff --git a/daemon/ls.c b/daemon/ls.c index 96e7bf287..d3689cdc8 100644 --- a/daemon/ls.c +++ b/daemon/ls.c @@ -131,7 +131,7 @@ do_llz (const char *path) int r; char *out; CLEANUP_FREE char *err = NULL; - CLEANUP_FREE char *spath; + CLEANUP_FREE char *spath = NULL; spath = sysroot_path (path); if (!spath) { diff --git a/daemon/xattr.c b/daemon/xattr.c index b0a818b2f..16a01a670 100644 --- a/daemon/xattr.c +++ b/daemon/xattr.c @@ -280,7 +280,7 @@ do_internal_lxattrlist (const char *path, char *const *names) for (k = 0; names[k] != NULL; ++k) { void *newptr; - CLEANUP_FREE char *pathname; + CLEANUP_FREE char *pathname = NULL; /* Be careful in this loop about which errors cause the whole call * to abort, and which errors allow us to continue processing diff --git a/fish/copy.c b/fish/copy.c index 477bfe986..07a36a2fc 100644 --- a/fish/copy.c +++ b/fish/copy.c @@ -217,7 +217,7 @@ run_copy_out (const char *cmd, size_t argc, char *argv[]) /* Download each remote one at a time using tar-out. */ int i, r; for (i = 0; i < nr_remotes; ++i) { - CLEANUP_FREE char *remote; + CLEANUP_FREE char *remote = NULL; /* Allow win:... prefix on remotes. */ remote = win_prefix (argv[i]); diff --git a/fish/inspect.c b/fish/inspect.c index 12ceaa75a..801d8673d 100644 --- a/fish/inspect.c +++ b/fish/inspect.c @@ -161,7 +161,7 @@ print_inspect_prompt (void) { size_t i; CLEANUP_FREE char *name = NULL; - CLEANUP_FREE_STRING_LIST char **mountpoints; + CLEANUP_FREE_STRING_LIST char **mountpoints = NULL; name = guestfs_inspect_get_product_name (g, root); if (name && STRNEQ (name, "unknown")) diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c index 6fa7c8c72..60b081dfa 100644 --- a/src/inspect-fs-unix.c +++ b/src/inspect-fs-unix.c @@ -892,7 +892,7 @@ check_fstab (guestfs_h *g, struct inspect_fs *fs) CLEANUP_FREE_STRING_LIST char **entries = NULL; char **entry; char augpath[256]; - CLEANUP_HASH_FREE Hash_table *md_map; + CLEANUP_HASH_FREE Hash_table *md_map = NULL; /* Generate a map of MD device paths listed in /etc/mdadm.conf to MD device * paths in the guestfs appliance */ diff --git a/src/inspect-fs-windows.c b/src/inspect-fs-windows.c index ba062eb83..4f4911221 100644 --- a/src/inspect-fs-windows.c +++ b/src/inspect-fs-windows.c @@ -480,7 +480,7 @@ check_windows_system_registry (guestfs_h *g, struct inspect_fs *fs) if (STRCASEEQLEN (key, "\\DosDevices\\", 12) && c_isalpha (key[12]) && key[13] == ':') { /* Get the binary value. Is it a fixed disk? */ - CLEANUP_FREE char *blob; + CLEANUP_FREE char *blob = NULL; char *device; size_t len; int64_t type; diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c index 8995d0461..23070683c 100644 --- a/src/launch-libvirt.c +++ b/src/launch-libvirt.c @@ -1621,7 +1621,7 @@ make_drive_priv (guestfs_h *g, struct drive *drv, drv_priv->format = drv->format ? safe_strdup (g, drv->format) : NULL; } else { - CLEANUP_FREE char *qemu_device; + CLEANUP_FREE char *qemu_device = NULL; drv_priv->real_src.protocol = drive_protocol_file; qemu_device = guestfs___drive_source_qemu_param (g, &drv->src);