[Libguestfs] [PATCH 66/67] Initialize CLEANUP_* stack variables with NULL in various places.

Richard W.M. Jones rjones at redhat.com
Sat Aug 24 11:05:06 UTC 2013


From: "Richard W.M. Jones" <rjones at redhat.com>

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.

(cherry picked from commit b1919066ca3d83f11b72d38d64f25739bd0ff67e)
---
 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 6e86bfc..434a7b4 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 b2d1eb2..83e2739 100644
--- a/daemon/augeas.c
+++ b/daemon/augeas.c
@@ -373,7 +373,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 8d10316..1ab76b0 100644
--- a/daemon/debug.c
+++ b/daemon/debug.c
@@ -321,7 +321,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),
@@ -391,7 +391,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";
@@ -418,7 +418,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 d1ce020..4fb6ee2 100644
--- a/daemon/ext2.c
+++ b/daemon/ext2.c
@@ -643,9 +643,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 9c29ec3..7f39e62 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 343c489..74228e5 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -664,7 +664,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;
@@ -708,7 +708,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 e26d984..ea34000 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 96e7bf2..d3689cd 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 b0a818b..16a01a6 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 477bfe9..07a36a2 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 12ceaa7..801d867 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 3f06310..5c517e1 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 ba062eb..4f49112 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 de725e0..6fcb880 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -1602,7 +1602,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);
-- 
1.8.3.1




More information about the Libguestfs mailing list