xfs
[Top] [All Lists]

Re: [PATCH 03/14] xfsprogs: trivial buffer frees on error paths

To: Eric Sandeen <sandeen@xxxxxxxxxx>
Subject: Re: [PATCH 03/14] xfsprogs: trivial buffer frees on error paths
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Fri, 11 Apr 2014 15:03:05 -0400
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1396999504-13769-4-git-send-email-sandeen@xxxxxxxxxx>
References: <1396999504-13769-1-git-send-email-sandeen@xxxxxxxxxx> <1396999504-13769-4-git-send-email-sandeen@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Apr 08, 2014 at 06:24:53PM -0500, Eric Sandeen wrote:
> Lots of memory leaks on error paths etc, spotted by
> coverity.  This patch rolls up the super-straightforward
> fixes across xfsprogs.
> 
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
>  db/addr.c    |   15 +++++++--------
>  db/check.c   |    4 +++-
>  db/write.c   |    2 ++
>  io/parent.c  |    2 ++
>  mkfs/proto.c |    3 ++-
>  5 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/db/addr.c b/db/addr.c
> index f74dd62..f9f760d 100644
> --- a/db/addr.c
> +++ b/db/addr.c
> @@ -85,16 +85,14 @@ addr_f(
>       fl = flist_scan(argv[1]);
>       if (fl == NULL)
>               return 0;
> -     if (!flist_parse(fld, fl, iocur_top->data, 0)) {
> -             flist_free(fl);
> -             return 0;
> -     }
> +     if (!flist_parse(fld, fl, iocur_top->data, 0))
> +             goto out;
> +     

You've got some trailing whitespace here (tab in the empty line).

Brian

>       flist_print(fl);
>       for (tfl = fl; tfl->child != NULL; tfl = tfl->child) {
>               if ((tfl->flags & FL_OKLOW) && tfl->low < tfl->high) {
>                       dbprintf(_("array not allowed for addr command\n"));
> -                     flist_free(fl);
> -                     return 0;
> +                     goto out;
>               }
>       }
>       fld = tfl->fld;
> @@ -103,7 +101,7 @@ addr_f(
>               next = inode_next_type();
>       if (next == TYP_NONE) {
>               dbprintf(_("no next type for field %s\n"), fld->name);
> -             return 0;
> +             goto out;
>       }
>       fa = &ftattrtab[fld->ftyp];
>       ASSERT(fa->ftyp == fld->ftyp);
> @@ -111,9 +109,10 @@ addr_f(
>       if (adf == NULL) {
>               dbprintf(_("no addr function for field %s (type %s)\n"),
>                       fld->name, fa->name);
> -             return 0;
> +             goto out;
>       }
>       (*adf)(iocur_top->data, tfl->offset, next);
> +out:
>       flist_free(fl);
>       return 0;
>  }
> diff --git a/db/check.c b/db/check.c
> index 4867698..baf7f9f 100644
> --- a/db/check.c
> +++ b/db/check.c
> @@ -1136,7 +1136,7 @@ blocktrash_f(
>       }
>       if (blocks == 0) {
>               dbprintf(_("blocktrash: no matching blocks\n"));
> -             return 0;
> +             goto out;
>       }
>       if (!sopt)
>               dbprintf(_("blocktrash: seed %u\n"), seed);
> @@ -1161,6 +1161,7 @@ blocktrash_f(
>                       }
>               }
>       }
> +out:
>       xfree(lentab);
>       return 0;
>  }
> @@ -1907,6 +1908,7 @@ ncheck_f(
>                       break;
>               default:
>                       dbprintf(_("bad option -%c for ncheck command\n"), c);
> +                     xfree(ilist);
>                       return 0;
>               }
>       }
> diff --git a/db/write.c b/db/write.c
> index 7b34fc0..ca8bd0f 100644
> --- a/db/write.c
> +++ b/db/write.c
> @@ -233,6 +233,7 @@ bwrite_lrot(
>       memcpy(hold_region, base, shift);
>       memcpy(base, base+shift, len-shift);
>       memcpy(base+(len-shift), hold_region, shift);
> +     free(hold_region);
>  }
>  
>  /* ARGSUSED */
> @@ -265,6 +266,7 @@ bwrite_rrot(
>       memcpy(hold_region, base+(len-shift), shift);
>       memmove(base+shift, base, len-shift);
>       memcpy(base, hold_region, shift);
> +     free(hold_region);
>  }
>  
>  /* ARGSUSED */
> diff --git a/io/parent.c b/io/parent.c
> index 47faaa0..ca989e9 100644
> --- a/io/parent.c
> +++ b/io/parent.c
> @@ -258,6 +258,8 @@ parent_check(void)
>       if (!bstatbuf || !parentbuf) {
>               fprintf(stderr, _("unable to allocate buffers: %s\n"),
>                       strerror(errno));
> +             free(bstatbuf);
> +             free(parentbuf);
>               return 1;
>       }
>  
> diff --git a/mkfs/proto.c b/mkfs/proto.c
> index b7e0761..95583c9 100644
> --- a/mkfs/proto.c
> +++ b/mkfs/proto.c
> @@ -49,7 +49,7 @@ char *
>  setup_proto(
>       char    *fname)
>  {
> -     char            *buf;
> +     char            *buf = NULL;
>       static char     dflt[] = "d--755 0 0 $";
>       int             fd;
>       long            size;
> @@ -85,6 +85,7 @@ setup_proto(
>  
>  out_fail:
>       close(fd);
> +     free(buf);
>       exit(1);
>  }
>  
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>