[1/2] btrfs-progs: receive: get rid of unnecessary strdup()
diff mbox series

Message ID f0142166d2059ed0bf319778dd3146d1d0b4523d.1563600688.git.osandov@fb.com
State New
Headers show
Series
  • btrfs-progs: fix clone from wrong subvolume
Related show

Commit Message

Omar Sandoval July 20, 2019, 5:40 a.m. UTC
From: Omar Sandoval <osandov@fb.com>

In process_clone(), we're not checking the return value of strdup().
But, there's no reason to strdup() in the first place: we just pass the
path into path_cat_out(). Get rid of the strdup().

Fixes: f1c24cd80dfd ("Btrfs-progs: add btrfs send/receive commands")
Signed-off-by: Omar Sandoval <osandov@osandov.com>
---
 cmds/receive.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

Omar Sandoval July 20, 2019, 5:42 a.m. UTC | #1
On Fri, Jul 19, 2019 at 10:40:00PM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> In process_clone(), we're not checking the return value of strdup().
> But, there's no reason to strdup() in the first place: we just pass the
> path into path_cat_out(). Get rid of the strdup().
> 
> Fixes: f1c24cd80dfd ("Btrfs-progs: add btrfs send/receive commands")
> Signed-off-by: Omar Sandoval <osandov@osandov.com>

Wrong SOB on this one, should be the usual

Signed-off-by: Omar Sandoval <osandov@fb.com>
Mike Fleetwood July 20, 2019, 8:34 a.m. UTC | #2
On Sat, 20 Jul 2019 at 06:43, Omar Sandoval <osandov@osandov.com> wrote:
>
> From: Omar Sandoval <osandov@fb.com>
>
> In process_clone(), we're not checking the return value of strdup().
> But, there's no reason to strdup() in the first place: we just pass the
> path into path_cat_out(). Get rid of the strdup().
>
> Fixes: f1c24cd80dfd ("Btrfs-progs: add btrfs send/receive commands")
> Signed-off-by: Omar Sandoval <osandov@osandov.com>
> ---
>  cmds/receive.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/cmds/receive.c b/cmds/receive.c
> index b97850a7..a3e62985 100644
> --- a/cmds/receive.c
> +++ b/cmds/receive.c
> @@ -739,7 +739,7 @@ static int process_clone(const char *path, u64 offset, u64 len,
>         struct btrfs_ioctl_clone_range_args clone_args;
>         struct subvol_info *si = NULL;
>         char full_path[PATH_MAX];
> -       char *subvol_path = NULL;
> +       char *subvol_path;
I think that should become const char *.

>         char full_clone_path[PATH_MAX];
>         int clone_fd = -1;
>
> @@ -760,7 +760,7 @@ static int process_clone(const char *path, u64 offset, u64 len,
>                 if (memcmp(clone_uuid, rctx->cur_subvol.received_uuid,
>                                 BTRFS_UUID_SIZE) == 0) {
>                         /* TODO check generation of extent */
> -                       subvol_path = strdup(rctx->cur_subvol_path);
> +                       subvol_path = rctx->cur_subvol_path;
>                 } else {
>                         if (!si)
>                                 ret = -ENOENT;
> @@ -794,14 +794,14 @@ static int process_clone(const char *path, u64 offset, u64 len,
>                         if (sub_len > root_len &&
>                             strstr(si->path, rctx->full_root_path) == si->path &&
>                             si->path[root_len] == '/') {
> -                               subvol_path = strdup(si->path + root_len + 1);
> +                               subvol_path = si->path + root_len + 1;
>                         } else {
>                                 error("clone: source subvol path %s unreachable from %s",
>                                         si->path, rctx->full_root_path);
>                                 goto out;
>                         }
>                 } else {
> -                       subvol_path = strdup(si->path);
> +                       subvol_path = si->path;
>                 }
>         }
>
> @@ -839,7 +839,6 @@ out:
>                 free(si->path);
>                 free(si);
>         }
> -       free(subvol_path);
>         if (clone_fd != -1)
>                 close(clone_fd);
>         return ret;
> --
> 2.22.0
>

Mike
Omar Sandoval July 21, 2019, 3:17 a.m. UTC | #3
On Sat, Jul 20, 2019 at 09:34:24AM +0100, Mike Fleetwood wrote:
> On Sat, 20 Jul 2019 at 06:43, Omar Sandoval <osandov@osandov.com> wrote:
> >
> > From: Omar Sandoval <osandov@fb.com>
> >
> > In process_clone(), we're not checking the return value of strdup().
> > But, there's no reason to strdup() in the first place: we just pass the
> > path into path_cat_out(). Get rid of the strdup().
> >
> > Fixes: f1c24cd80dfd ("Btrfs-progs: add btrfs send/receive commands")
> > Signed-off-by: Omar Sandoval <osandov@osandov.com>
> > ---
> >  cmds/receive.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/cmds/receive.c b/cmds/receive.c
> > index b97850a7..a3e62985 100644
> > --- a/cmds/receive.c
> > +++ b/cmds/receive.c
> > @@ -739,7 +739,7 @@ static int process_clone(const char *path, u64 offset, u64 len,
> >         struct btrfs_ioctl_clone_range_args clone_args;
> >         struct subvol_info *si = NULL;
> >         char full_path[PATH_MAX];
> > -       char *subvol_path = NULL;
> > +       char *subvol_path;
> I think that should become const char *.

Yeah, that wouldn't hurt. Dave, can you add that when you apply this or
should I resend?

Patch
diff mbox series

diff --git a/cmds/receive.c b/cmds/receive.c
index b97850a7..a3e62985 100644
--- a/cmds/receive.c
+++ b/cmds/receive.c
@@ -739,7 +739,7 @@  static int process_clone(const char *path, u64 offset, u64 len,
 	struct btrfs_ioctl_clone_range_args clone_args;
 	struct subvol_info *si = NULL;
 	char full_path[PATH_MAX];
-	char *subvol_path = NULL;
+	char *subvol_path;
 	char full_clone_path[PATH_MAX];
 	int clone_fd = -1;
 
@@ -760,7 +760,7 @@  static int process_clone(const char *path, u64 offset, u64 len,
 		if (memcmp(clone_uuid, rctx->cur_subvol.received_uuid,
 				BTRFS_UUID_SIZE) == 0) {
 			/* TODO check generation of extent */
-			subvol_path = strdup(rctx->cur_subvol_path);
+			subvol_path = rctx->cur_subvol_path;
 		} else {
 			if (!si)
 				ret = -ENOENT;
@@ -794,14 +794,14 @@  static int process_clone(const char *path, u64 offset, u64 len,
 			if (sub_len > root_len &&
 			    strstr(si->path, rctx->full_root_path) == si->path &&
 			    si->path[root_len] == '/') {
-				subvol_path = strdup(si->path + root_len + 1);
+				subvol_path = si->path + root_len + 1;
 			} else {
 				error("clone: source subvol path %s unreachable from %s",
 					si->path, rctx->full_root_path);
 				goto out;
 			}
 		} else {
-			subvol_path = strdup(si->path);
+			subvol_path = si->path;
 		}
 	}
 
@@ -839,7 +839,6 @@  out:
 		free(si->path);
 		free(si);
 	}
-	free(subvol_path);
 	if (clone_fd != -1)
 		close(clone_fd);
 	return ret;