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

[Richard W.M. Jones]

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 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
[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