Re: [PATCH 5/5] btrfs-progs:free strdup()s that are not freed

[Wang Shilong]

On 09/05/2013 04:08 PM, Gui Hecheng wrote:

On Thu, 2013-09-05 at 09:10 +0200, Stefan Behrens wrote:

On Thu, 05 Sep 2013 09:00:07 +0200, Stefan Behrens wrote:

On Thu, 5 Sep 2013 10:38:58 +0800, Gui Hecheng wrote:

The strdup()s not freed are reported as memory leaks by valgrind.

Signed-off-by: Gui Hecheng <guihc.f...@cn.fujitsu.com>
---
  cmds-subvolume.c | 48 ++++++++++++++++++++++++++++++++++--------------
  1 file changed, 34 insertions(+), 14 deletions(-)

Just noticed that you had already sent a V2 with the things fixed that I
have commented, but you sent the 5/5 as a 1/5 and added the changelog
v1->v2 "none" which made it difficult to recognize :). But maybe David
Sterba is smart enough when he picks up the patches.

Thank you for your friendly advice. I will handle it snow.

            s/snow/soon ^_^

Thanks,
Wang

diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index e1fa81a..51c529c 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c

[...]

@@ -75,6 +75,8 @@ static int cmd_subvol_create(int argc, char **argv)
  {
        int     retval, res, len;
        int     fddst = -1;
+       char    *dupname = NULL;
+       char    *dupdir = NULL;
        char    *newname;
        char    *dstdir;
        char    *dst;
@@ -119,10 +121,10 @@ static int cmd_subvol_create(int argc, char **argv)
                goto out;
        }

- newname = strdup(dst);

-       newname = basename(newname);
-       dstdir = strdup(dst);
-       dstdir = dirname(dstdir);
+       dupname = strdup(dst);
+       newname = basename(dupname);
+       dupdir = strdup(dst);
+       dstdir = dirname(dupdir);

if (!strcmp(newname, ".") || !strcmp(newname, "..") ||

             strchr(newname, '/') ){
@@ -175,6 +177,11 @@ out:
        close_file_or_dir(fddst, dirstream);
        free(inherit);

+ if (dupname != NULL)

+               free(dupname);
+       if (dupdir != NULL)
+               free(dupdir);
+

free(3) already checks the pointer for NULL, no need to do it on your own.

        return retval;
  }

@@ -208,6 +215,8 @@ static int cmd_subvol_delete(int argc, char **argv)

        int     res, fd, len, e, cnt = 1, ret = 0;
        struct btrfs_ioctl_vol_args     args;
        char    *dname, *vname, *cpath;
+       char    *dupdname = NULL;
+       char    *dupvname = NULL;
        char    *path;
        DIR     *dirstream = NULL;

@@ -230,10 +239,10 @@ again:

        }

cpath = realpath(path, NULL);

-       dname = strdup(cpath);
-       dname = dirname(dname);
-       vname = strdup(cpath);
-       vname = basename(vname);
+       dupdname = strdup(cpath);
+       dname = dirname(dupdname);
+       dupvname = strdup(cpath);
+       vname = basename(dupvname);
        free(cpath);

if( !strcmp(vname,".") || !strcmp(vname,"..") ||

@@ -274,6 +283,10 @@ again:
        }

out:

+       if (dupdname != NULL)
+               free(dupdname);
+       if (dupvname != NULL)
+               free(dupvname);

Here again.

        cnt++;
        if (cnt < argc)
                goto again;
@@ -495,6 +508,8 @@ static int cmd_snapshot(int argc, char **argv)
        int     res, retval;
        int     fd = -1, fddst = -1;
        int     len, readonly = 0;
+       char    *dupname = NULL;
+       char    *dupdir = NULL;
        char    *newname;
        char    *dstdir;
        struct btrfs_ioctl_vol_args_v2  args;
@@ -562,14 +577,14 @@ static int cmd_snapshot(int argc, char **argv)
        }

if (res > 0) {

-               newname = strdup(subvol);
-               newname = basename(newname);
+               dupname = strdup(subvol);
+               newname = basename(dupname);
                dstdir = dst;
        } else {
-               newname = strdup(dst);
-               newname = basename(newname);
-               dstdir = strdup(dst);
-               dstdir = dirname(dstdir);
+               dupname = strdup(dst);
+               newname = basename(dupname);
+               dupdir = strdup(dst);
+               dstdir = dirname(dupdir);
        }

if (!strcmp(newname, ".") || !strcmp(newname, "..") ||

@@ -630,6 +645,11 @@ out:
        close_file_or_dir(fd, dirstream2);
        free(inherit);

+ if (dupname != NULL)

+               free(dupname);
+       if (dupdir != NULL)
+               free(dupdir);
+

And here.

        return retval;
  }

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html