[fedora-virt] [PATCH] add comments suggesting memory-handling improvements

Jim Meyering jim at meyering.net
Wed Jul 1 14:11:50 UTC 2009


Hi Rich,

Looking at the sole remaining uses of asprintf, I noticed these
and some realloc/strdup results that needed to be checked.

Re asprintf, note that one must always check it for failure,
and when it fails, you cannot use the (undefined) pointer result.
Obviously this patch is just for reference, i.e, not to apply.

>From 7b7df29516dad78a3c815ea1ef301da89d4b5323 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Wed, 1 Jul 2009 16:09:33 +0200
Subject: [PATCH] add comments suggesting memory-handling improvements

* fish/destpaths.c: Don't deref NULL or undef on failed heap alloc.
---
 fish/destpaths.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/fish/destpaths.c b/fish/destpaths.c
index 6cddafa..a5ece2a 100644
--- a/fish/destpaths.c
+++ b/fish/destpaths.c
@@ -55,7 +55,7 @@ complete_dest_paths_generator (const char *text, int state)

   static int len, index;
   static char **words = NULL;
-  static int nr_words = 0;
+  static int nr_words = 0; // nr_words should be size_t
   char *word;
   guestfs_error_handler_cb old_error_cb;
   void *old_error_cb_data;
@@ -75,6 +75,7 @@ complete_dest_paths_generator (const char *text, int state)
     char **strs;
     int i, n;

+    // len should be of type size_t; index and "i", too
     len = strlen (text);
     index = 0;

@@ -92,8 +93,11 @@ complete_dest_paths_generator (const char *text, int state)

 #define APPEND_STRS_AND_FREE						\
     if (strs) {								\
+      // n should be unsigned, like size_t
       n = count_strings (strs);						\
+      // handle the case in which sizeof (char *) * (nr_words + n)) overflows
       words = realloc (words, sizeof (char *) * (nr_words + n));	\
+      // handle failed realloc
       for (i = 0; i < n; ++i)						\
 	words[nr_words++] = strs[i];					\
       free (strs);							\
@@ -120,17 +124,20 @@ complete_dest_paths_generator (const char *text, int state)

       p = strrchr (text, '/');
       dir = p && p > text ? strndup (text, p - text) : strdup ("/");
+      // handle failed strdup

       strs = guestfs_ls (g, dir);

       /* Prepend directory to names. */
       if (strs) {
 	for (i = 0; strs[i]; ++i) {
+	  int err;
 	  p = NULL;
 	  if (strcmp (dir, "/") == 0)
-	    asprintf (&p, "/%s", strs[i]);
+	    err = asprintf (&p, "/%s", strs[i]);
 	  else
-	    asprintf (&p, "%s/%s", dir, strs[i]);
+	    err = asprintf (&p, "%s/%s", dir, strs[i]);
+	  // handle failed asprintf, i.e., err < 0
 	  free (strs[i]);
 	  strs[i] = p;
 	}
--
1.6.3.3.483.g4f5e




More information about the Fedora-virt mailing list