Re: [fedora-virt] [PATCH] add comments suggesting memory-handling improvements
On Wed, Jul 01, 2009 at 04:11:50PM +0200, Jim Meyering wrote:
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(a)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
[etc]
Was the plan to add these comments to the source now in the hope
we'd fix it in future?
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.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