Re: [LTP] [PATCH 2/3] fsstress: fix memory leak in fread_d

[Garrett Cooper <yanegomi@gmail.com>]

On Sat, Sep 26, 2009 at 4:34 AM, Andrew Vagin <avagin@gmail.com> wrote:
> 1. check result from operation open before set flags
> 2. close descriptor and free path if set flags failed
>
> valgrind --leak-check=full --show-reachable=yes ./fsstress -d /home/shpagin/git-archive/ltp/testcases/kernel/fs/fsstress/tmp -l 10 -n 100
>
> ==23212== 1,134 bytes in 108 blocks are definitely lost in loss record 4 of 5
> ==23212==    at 0x4A0776F: realloc (vg_replace_malloc.c:429)
> ==23212==    by 0x401E6A: append_pathname (fsstress.c:528)
> ==23212==    by 0x4024E3: fent_to_name (fsstress.c:760)
> ==23212==    by 0x40284F: get_fname (fsstress.c:846)
> ==23212==    by 0x4041E0: dread_f (fsstress.c:1757)
> ==23212==    by 0x402368: doproc (fsstress.c:728)
> ==23212==    by 0x401CC1: main (fsstress.c:463)
>
>
> ---
>  testcases/kernel/fs/fsstress/fsstress.c |   16 +++++++++++-----
>  1 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/testcases/kernel/fs/fsstress/fsstress.c b/testcases/kernel/fs/fsstress/fsstress.c
> index 3df0ce5..db6b358 100644
> --- a/testcases/kernel/fs/fsstress/fsstress.c
> +++ b/testcases/kernel/fs/fsstress/fsstress.c
> @@ -1764,10 +1764,6 @@ dread_f(int opno, long r)
>        }
>        fd = open_path(&f, O_RDONLY);
>
> -       if (!setdirect(fd)) {
> -               return;
> -       }
> -
>        e = fd < 0 ? errno : 0;
>        check_cwd();
>        if (fd < 0) {
> @@ -1777,6 +1773,13 @@ dread_f(int opno, long r)
>                free_pathname(&f);
>                return;
>        }
> +
> +       if (!setdirect(fd)) {
> +               close(fd);
> +               free_pathname(&f);
> +               return;
> +       }
> +
>        if (fstat64(fd, &stb) < 0) {
>                if (v)
>                        printf("%d/%d: dread - fstat64 %s failed %d\n",
> @@ -1864,8 +1867,11 @@ dwrite_f(int opno, long r)
>                return;
>        }
>
> -       if (!setdirect(fd))
> +       if (!setdirect(fd)) {
> +               free_pathname(&f);
> +               close(fd);
>                return;
> +       }
>        if (fstat64(fd, &stb) < 0) {
>                if (v)
>                        printf("%d/%d: dwrite - fstat64 %s failed %d\n",

Andrew,
    This change looks good, but the indentation is a bit catywampus.
Please fix and run with checkpatch.pl.
Thanks!
-Garrett

------------------------------------------------------------------------------
Come build with us! The BlackBerry&reg; Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay 
ahead of the curve. Join us from November 9&#45;12, 2009. Register now&#33;
http://p.sf.net/sfu/devconf
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list