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 <[email protected]>
> >> ---
> >>  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.

> >>
> >> 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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to