| Message ID | 162528108811.36401.13142861358282476701.stgit@locust (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs_io: small fixes to funshare command | expand |
On Fri, Jul 02, 2021 at 07:58:08PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Add proper argument parsing to the funshare command so that when you > pass it nonexistent --help it will print the help instead of complaining > that it can't convert that to a number. > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > io/prealloc.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > > diff --git a/io/prealloc.c b/io/prealloc.c > index 2ae8afe9..94cf326f 100644 > --- a/io/prealloc.c > +++ b/io/prealloc.c > @@ -346,16 +346,24 @@ funshare_f( > char **argv) > { > xfs_flock64_t segment; > + int c; > int mode = FALLOC_FL_UNSHARE_RANGE; > - int index = 1; > > - if (!offset_length(argv[index], argv[index + 1], &segment)) { > + while ((c = getopt(argc, argv, "")) != EOF) { > + switch (c) { > + default: > + command_usage(&funshare_cmd); > + } Do we really need this switch boilerplate? > + } > + if (optind != argc - 2) > + return command_usage(&funshare_cmd); Spaces instead of tabs here.
On Mon, Jul 05, 2021 at 04:20:06PM +0100, Christoph Hellwig wrote: > On Fri, Jul 02, 2021 at 07:58:08PM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@kernel.org> > > > > Add proper argument parsing to the funshare command so that when you > > pass it nonexistent --help it will print the help instead of complaining > > that it can't convert that to a number. > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > --- > > io/prealloc.c | 16 ++++++++++++---- > > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > > > diff --git a/io/prealloc.c b/io/prealloc.c > > index 2ae8afe9..94cf326f 100644 > > --- a/io/prealloc.c > > +++ b/io/prealloc.c > > @@ -346,16 +346,24 @@ funshare_f( > > char **argv) > > { > > xfs_flock64_t segment; > > + int c; > > int mode = FALLOC_FL_UNSHARE_RANGE; > > - int index = 1; > > > > - if (!offset_length(argv[index], argv[index + 1], &segment)) { > > + while ((c = getopt(argc, argv, "")) != EOF) { > > + switch (c) { > > + default: > > + command_usage(&funshare_cmd); > > + } > > Do we really need this switch boilerplate? Sort of -- without it, you get interactions like: $ xfs_io -c 'funshare 0 --help' /autoexec.bat non-numeric length-argument -- --help which is silly, since we could display the help screen. The other solution might be to fix all the offset_length() callers to emit command usage when the number parsing doesn't work, but that would clutter up the error reporting when you try to feed it a number that doesn't parse. > > + } > > + if (optind != argc - 2) > > + return command_usage(&funshare_cmd); > > Spaces instead of tabs here. Fixed. --D
On Fri, Jul 02, 2021 at 07:58:08PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Add proper argument parsing to the funshare command so that when you > pass it nonexistent --help it will print the help instead of complaining > that it can't convert that to a number. Looks ok to me despite the space/tab thing already mentioned by hch. Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > io/prealloc.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > > diff --git a/io/prealloc.c b/io/prealloc.c > index 2ae8afe9..94cf326f 100644 > --- a/io/prealloc.c > +++ b/io/prealloc.c > @@ -346,16 +346,24 @@ funshare_f( > char **argv) > { > xfs_flock64_t segment; > + int c; > int mode = FALLOC_FL_UNSHARE_RANGE; > - int index = 1; > > - if (!offset_length(argv[index], argv[index + 1], &segment)) { > + while ((c = getopt(argc, argv, "")) != EOF) { > + switch (c) { > + default: > + command_usage(&funshare_cmd); > + } > + } > + if (optind != argc - 2) > + return command_usage(&funshare_cmd); > + > + if (!offset_length(argv[optind], argv[optind + 1], &segment)) { > exitcode = 1; > return 0; > } > > - if (fallocate(file->fd, mode, > - segment.l_start, segment.l_len)) { > + if (fallocate(file->fd, mode, segment.l_start, segment.l_len)) { > perror("fallocate"); > exitcode = 1; > return 0; >
diff --git a/io/prealloc.c b/io/prealloc.c index 2ae8afe9..94cf326f 100644 --- a/io/prealloc.c +++ b/io/prealloc.c @@ -346,16 +346,24 @@ funshare_f( char **argv) { xfs_flock64_t segment; + int c; int mode = FALLOC_FL_UNSHARE_RANGE; - int index = 1; - if (!offset_length(argv[index], argv[index + 1], &segment)) { + while ((c = getopt(argc, argv, "")) != EOF) { + switch (c) { + default: + command_usage(&funshare_cmd); + } + } + if (optind != argc - 2) + return command_usage(&funshare_cmd); + + if (!offset_length(argv[optind], argv[optind + 1], &segment)) { exitcode = 1; return 0; } - if (fallocate(file->fd, mode, - segment.l_start, segment.l_len)) { + if (fallocate(file->fd, mode, segment.l_start, segment.l_len)) { perror("fallocate"); exitcode = 1; return 0;