| Message ID | fe818d07-f539-4b2c-fe26-dbc18003e3e2@sandeen.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xfs_io: reorganize source file handling in copy_range | expand |
On Wed, Jun 26, 2019 at 5:51 PM Eric Sandeen <sandeen@sandeen.net> wrote: > > rename and rearrange some of the vars related to using an open > file number as the source file, so that we don't temporarily > store a non-fd number in a var called "fd," and do the fd > assignment in a consistent code location. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > > Amir, what do you think about this tweak, just for maintainability > I'd prefer to only assign an actual fd to "fd," and handle that > assignment in the same code location for both invocation options. > Thoughts? Not a big deal either way. I think that looks much better. ... > + } else > + fd = filetable[src_file_nr].fd; I would match else { } to if { }. Other than that, looks good. You may add: Reviewed-by: Amir Goldstein <amir73il@gmail.com> Thanks, Amir.
diff --git a/io/copy_file_range.c b/io/copy_file_range.c index 4a0e7a77..90e1452c 100644 --- a/io/copy_file_range.c +++ b/io/copy_file_range.c @@ -84,7 +84,8 @@ copy_range_f(int argc, char **argv) int opt; int ret; int fd; - int src_file_arg = 1; + int src_path_arg = 1; + int src_file_nr = 0; size_t fsblocksize, fssectsize; init_cvtnum(&fsblocksize, &fssectsize); @@ -113,27 +114,27 @@ copy_range_f(int argc, char **argv) } break; case 'f': - fd = atoi(argv[1]); - if (fd < 0 || fd >= filecount) { + src_file_nr = atoi(argv[1]); + if (src_file_nr < 0 || src_file_nr >= filecount) { printf(_("value %d is out of range (0-%d)\n"), - fd, filecount-1); + src_file_nr, filecount-1); return 0; } - fd = filetable[fd].fd; - /* Expect no src_file arg */ - src_file_arg = 0; + /* Expect no src_path arg */ + src_path_arg = 0; break; } } - if (optind != argc - src_file_arg) + if (optind != argc - src_path_arg) return command_usage(©_range_cmd); - if (src_file_arg) { + if (src_path_arg) { fd = openfile(argv[optind], NULL, IO_READONLY, 0, NULL); if (fd < 0) return 0; - } + } else + fd = filetable[src_file_nr].fd; if (src == 0 && dst == 0 && len == 0) { off64_t sz;
rename and rearrange some of the vars related to using an open file number as the source file, so that we don't temporarily store a non-fd number in a var called "fd," and do the fd assignment in a consistent code location. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- Amir, what do you think about this tweak, just for maintainability I'd prefer to only assign an actual fd to "fd," and handle that assignment in the same code location for both invocation options. Thoughts? Not a big deal either way.