| Message ID | f0142166d2059ed0bf319778dd3146d1d0b4523d.1563600688.git.osandov@fb.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: fix clone from wrong subvolume | expand |
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>
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
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?
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;