Re: [PATCH 05/19] mkfs: factor boolean option parsing

[Eric Sandeen <sandeen@sandeen.net>]

On 3/24/16 6:15 AM, jtulak@redhat.com wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> UPDATE:
> - add a forgotten getbool update to FINOBT
> - change -m crc and ftype to enable by specifying (instead of toggle off)
> 
> Many of the options passed to mkfs have boolean options (0 or 1) and
> all hand roll the same code and validity checks. Factor these out
> into a common function.
> 
> Note that the lazy-count option is now changed to match other
> booleans in that if you don't specify a value, it reverts to the
> default value (on) rather than throwing an error. 
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Jan Tulak <jtulak@redhat.com>
> ---
>  mkfs/xfs_mkfs.c | 109 +++++++++++++++++++++++---------------------------------
>  1 file changed, 44 insertions(+), 65 deletions(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 4811d77..9394bd3 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c

...

> @@ -1451,12 +1459,8 @@ main(
>  					sb_feat.attr_version = c;
>  					break;
>  				case I_PROJID32BIT:
> -					if (!value || *value == '\0')
> -						value = "0";
> -					c = getnum(value, 0, 0, false);
> -					if (c < 0 || c > 1)
> -						illegal(value, "i projid32bit");
> -					sb_feat.projid16bit = c ? false : true;
> +					sb_feat.projid16bit = !getbool(value,
> +							"i projid32bit", false);
>  					break;
>  				case I_SPINODES:
>  					if (!value || *value == '\0')

I_SPINODES now needs the getbool() treatment as well.

...

> @@ -1631,28 +1625,20 @@ main(
> 
>  				switch (getsubopt(&p, (constpp)mopts, &value)) {
>  				case M_CRC:
> -					if (!value || *value == '\0')
> -						reqval('m', mopts, M_CRC);
> -					c = getnum(value, 0, 0, false);
> -					if (c < 0 || c > 1)
> -						illegal(value, "m crc");
> -					if (c && nftype) {
> +					sb_feat.crcs_enabled = getbool(
> +							value, "m crc", true);
> +					if (sb_feat.crcs_enabled && nftype) {
>  						fprintf(stderr,
> -_("cannot specify both crc and ftype\n"));
> +_("cannot specify both -m crc=1 and -n ftype\n"));
>  						usage();
>  					}
> -					sb_feat.crcs_enabled = c ? true : false;
> -					if (c)
> +					if (sb_feat.crcs_enabled)
>  						sb_feat.dirftype = true;
>  					break;
>  				case M_FINOBT:
> -					if (!value || *value == '\0')
> -						reqval('m', mopts, M_CRC);

                                                                   ^^^ whoops ;)
Well that bug goes away now ;)

> -					c = atoi(value);
> -					if (c < 0 || c > 1)
> -						illegal(value, "m finobt");
>  					sb_feat.finobtflag = true;
> -					sb_feat.finobt = c;
> +					sb_feat.finobt = getbool(
> +						value, "m finobt", true);
>  					break;
>  				case M_UUID:
>  					if (!value || *value == '\0')

Other than SPINODES this one looks ok to me.

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs