Re: [PATCH 08/19] mkfs: getbool is redundant

Eric Sandeen
2016-04-07T17:25:54Z

On 3/24/16 6:15 AM, jtulak@redhat.com wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> CHANGELOG:
> o Add description of a new member of opt_params
> o Fix in the subopts tables
> o All boolean subopts should use getnum_checked
> o Edit man8 page where it missed the information about optional value of a flag
>
> getbool() can be replaced with getnum_checked with appropriate
> min/max values set for the boolean variables. Make boolean
> arguments consistent - all accept 0 or 1 value now.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Jan Tulak <jtulak@redhat.com>
> ---
> man/man8/mkfs.xfs.8 | 11 ++-
> mkfs/xfs_mkfs.c | 223 ++++++++++++++++++++++++++++++++++++----------------
> 2 files changed, 164 insertions(+), 70 deletions(-)
>
> diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8
> index 8b068ef..5a2a20a 100644
> --- a/man/man8/mkfs.xfs.8
> +++ b/man/man8/mkfs.xfs.8
> @@ -118,6 +118,9 @@ option or the
> option first needs to be added to the command line.
> Failure to specify the size of the units will result in illegal value errors
> when parameters are quantified in those units.
> +.PP
> +Many options allows for optional argument of value 0 or 1, to explicitly

"Many options allow for an optional argument of 0 or 1, ..."

> +disable or enable the functionality, in a forward-compatible syntax.

What does "forward-compatible syntax" mean? I'm not sure that clarifies
anything for the reader.

Otherwise this looks ok to me; Dave explained that it is intentional to
make every single option accept a value, whether it is now
boolean or a numeric value, so there is no such thing as a bare "--flag"
anymore; such flags are always "--flag [0|1]" now, right?

> @@ -312,10 +315,12 @@ and
> .B swidth
> values.
> .TP
> -.BI noalign
> -This option disables automatic geometry detection and creates the filesystem
> +.BI noalign[= value ]
> +If the
> +.I value
> +is 1, this option disables automatic geometry detection and creates the filesystem
> without stripe geometry alignment even if the underlying storage device provides
> -this information.
> +this information. 0 disables this option, that is, enables automatic detection.
> .RE
> .TP
> .B \-f

The realtime section of the manpage needs this same fixup now too, I guess.

Thanks,
-Eric