Re: [PATCH 2/2] xfs_io: clean up the funshare command a bit

["Darrick J. Wong" <djwong@kernel.org>]

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