[PATCH 08/19] mkfs: getbool is redundant

jtulak
2016-03-24T11:15:49Z

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
+disable or enable the functionality, in a forward-compatible syntax.
.SH OPTIONS
.TP
.BI \-b " block_size_options"
@@ -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
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 6e29848..0e2cfac 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -49,6 +49,7 @@ static long long cvtnum(unsigned int blocksize,
unsigned int sectorsize, const char *s);

#define MAX_SUBOPTS 16
+#define SUBOPT_NEEDS_VAL (-1LL)
/*
* Table for parsing mkfs parameters.
*
@@ -83,6 +84,13 @@ static long long cvtnum(unsigned int blocksize,
* (Said in another way, you can't have minval and maxval both equal
* to zero. But if one value is different: minval=0 and maxval=1,
* then it is OK.)
+ *
+ * defaultval MANDATORY
+ * The value used if user specifies the subopt, but no value.
+ * If the subopt accepts some values (-d file=[1|0]), then this
+ * sets what is used with simple specifying the subopt (-d file).
+ * A special SUBOPT_NEEDS_VAL can be used to require a user-given
+ * value in any case.
*/
struct opt_params {
const char name;
@@ -91,6 +99,7 @@ struct opt_params {
int index;
long long minval;
long long maxval;
+ long long defaultval;
} subopt_params[MAX_SUBOPTS];
};

@@ -107,10 +116,12 @@ struct opt_params bopts = {
{ .index = B_LOG,
.minval = XFS_MIN_BLOCKSIZE_LOG,
.maxval = XFS_MAX_BLOCKSIZE_LOG,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = B_SIZE,
.minval = XFS_MIN_BLOCKSIZE,
.maxval = XFS_MAX_BLOCKSIZE,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
},
};
@@ -152,38 +163,57 @@ struct opt_params dopts = {
},
.subopt_params = {
{ .index = D_AGCOUNT,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_FILE,
+ .minval = 0,
+ .maxval = 1,
+ .defaultval = 1,
},
{ .index = D_NAME,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_SIZE,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_SUNIT,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_SWIDTH,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_AGSIZE,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_SU,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_SW,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_SECTLOG,
.minval = XFS_MIN_SECTORSIZE_LOG,
.maxval = XFS_MAX_SECTORSIZE_LOG,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_SECTSIZE,
.minval = XFS_MIN_SECTORSIZE,
.maxval = XFS_MAX_SECTORSIZE,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_NOALIGN,
+ .minval = 0,
+ .maxval = 1,
+ .defaultval = 1,
},
{ .index = D_RTINHERIT,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_PROJINHERIT,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = D_EXTSZINHERIT,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
},
};
@@ -212,22 +242,36 @@ struct opt_params iopts = {
},
.subopt_params = {
{ .index = I_ALIGN,
+ .minval = 0,
+ .maxval = 1,
+ .defaultval = 1,
},
{ .index = I_LOG,
.minval = XFS_DINODE_MIN_LOG,
.maxval = XFS_DINODE_MAX_LOG,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = I_MAXPCT,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = I_PERBLOCK,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = I_SIZE,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = I_ATTR,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = I_PROJID32BIT,
+ .minval = 0,
+ .maxval = 1,
+ .defaultval = 1,
},
{ .index = I_SPINODES,
+ .minval = 0,
+ .maxval = 1,
+ .defaultval = 1,
},
},
};
@@ -263,32 +307,50 @@ struct opt_params lopts = {
},
.subopt_params = {
{ .index = L_AGNUM,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_INTERNAL,
+ .minval = 0,
+ .maxval = 1,
+ .defaultval = 1,
},
{ .index = L_SIZE,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_VERSION,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_SUNIT,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_SU,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_DEV,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_SECTLOG,
.minval = XFS_MIN_SECTORSIZE_LOG,
.maxval = XFS_MAX_SECTORSIZE_LOG,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_SECTSIZE,
.minval = XFS_MIN_SECTORSIZE,
.maxval = XFS_MAX_SECTORSIZE,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_FILE,
+ .minval = 0,
+ .maxval = 1,
+ .defaultval = 1,
},
{ .index = L_NAME,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = L_LAZYSBCNTR,
+ .minval = 0,
+ .maxval = 1,
+ .defaultval = 1,
},
},
};
@@ -310,14 +372,20 @@ struct opt_params nopts = {
{ .index = N_LOG,
.minval = XFS_MIN_REC_DIRSIZE,
.maxval = XFS_MAX_BLOCKSIZE_LOG,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = N_SIZE,
.minval = 1 << XFS_MIN_REC_DIRSIZE,
.maxval = XFS_MAX_BLOCKSIZE,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = N_VERSION,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = N_FTYPE,
+ .minval = 0,
+ .maxval = 1,
+ .defaultval = 1,
},
},
};
@@ -341,16 +409,26 @@ struct opt_params ropts = {
},
.subopt_params = {
{ .index = R_EXTSIZE,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = R_SIZE,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = R_DEV,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = R_FILE,
+ .minval = 0,
+ .maxval = 1,
+ .defaultval = 1,
},
{ .index = R_NAME,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = R_NOALIGN,
+ .minval = 0,
+ .maxval = 1,
+ .defaultval = 1,
},
},
};
@@ -372,18 +450,22 @@ struct opt_params sopts = {
{ .index = S_LOG,
.minval = XFS_MIN_SECTORSIZE_LOG,
.maxval = XFS_MAX_SECTORSIZE_LOG,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = S_SECTLOG,
.minval = XFS_MIN_SECTORSIZE_LOG,
.maxval = XFS_MAX_SECTORSIZE_LOG,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = S_SIZE,
.minval = XFS_MIN_SECTORSIZE,
.maxval = XFS_MAX_SECTORSIZE,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
{ .index = S_SECTSIZE,
.minval = XFS_MIN_SECTORSIZE,
.maxval = XFS_MAX_SECTORSIZE,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
},
};
@@ -394,13 +476,24 @@ struct opt_params mopts = {
#define M_CRC 0
"crc",
#define M_FINOBT 1
- "finobt",
+ "finobt",
#define M_UUID 2
"uuid",
NULL
},
.subopt_params = {
{ .index = M_CRC,
+ .minval = 0,
+ .maxval = 1,
+ .defaultval = 1,
+ },
+ { .index = M_FINOBT,
+ .minval = 0,
+ .maxval = 1,
+ .defaultval = 1,
+ },
+ { .index = M_UUID,
+ .defaultval = SUBOPT_NEEDS_VAL,
},
},
};
@@ -1220,22 +1313,6 @@ getnum(
return i;
}

-static bool
-getbool(
- const char *str,
- const char *illegal_str,
- bool default_val)
-{
- long long c;
-
- if (!str || *str == '\0')
- return default_val;
- c = getnum(str, 0, 0, false);
- if (c < 0 || c > 1)
- illegal(str, illegal_str);
- return c ? true : false;
-}
-
static __attribute__((noreturn)) void
illegal_option(
const char *value,
@@ -1250,17 +1327,27 @@ illegal_option(

static int
getnum_checked(
- const char *str,
+ const char *str,
struct opt_params *opts,
- int index)
+ int index)
{
- long long c;
+ const struct subopt_param *sp = &opts->subopt_params[index];
+ long long c;

- if (!str || *str == '\0')
+ if (sp->index != index) {
+ fprintf(stderr,
+ ("Developer screwed up option parsing (%d/%d)! Please report!\n"),
+ sp->index, index);
reqval(opts->name, (char **)opts->subopts, index);
+ }

- if (opts->subopt_params[index].minval == 0 &&
- opts->subopt_params[index].maxval == 0) {
+ if (!str || *str == '\0') {
+ if (sp->defaultval == SUBOPT_NEEDS_VAL)
+ reqval(opts->name, (char **)opts->subopts, index);
+ return sp->defaultval;
+ }
+
+ if (sp->minval == 0 && sp->maxval == 0) {
fprintf(stderr,
_("Option -%c %s has undefined minval/maxval."
"Can't verify value range. This is a bug.\n"),
@@ -1269,8 +1356,7 @@ getnum_checked(
}

c = getnum(str, 0, 0, false);
- if (c < opts->subopt_params[index].minval ||
- c > opts->subopt_params[index].maxval)
+ if (c < sp->minval || c > sp->maxval)
illegal_option(str, opts, index);
return c;
}
@@ -1495,8 +1581,8 @@ main(
dasize = 1;
break;
case D_FILE:
- xi.disfile = getbool(value, "d file",
- true);
+ xi.disfile = getnum_checked(value,
+ &dopts, D_FILE);
if (xi.disfile && !Nflag)
xi.dcreat = 1;
break;
@@ -1564,19 +1650,22 @@ main(
illegal(value, "d sw");
break;
case D_NOALIGN:
- if (dsu)
- conflict('d', subopts, D_SU,
- D_NOALIGN);
- if (dsunit)
- conflict('d', subopts, D_SUNIT,
- D_NOALIGN);
- if (dsw)
- conflict('d', subopts, D_SW,
- D_NOALIGN);
- if (dswidth)
- conflict('d', subopts, D_SWIDTH,
- D_NOALIGN);
- nodsflag = 1;
+ nodsflag = getnum_checked(value,
+ &dopts, D_NOALIGN);
+ if (nodsflag) {
+ if (dsu)
+ conflict('d', subopts, D_SU,
+ D_NOALIGN);
+ if (dsunit)
+ conflict('d', subopts, D_SUNIT,
+ D_NOALIGN);
+ if (dsw)
+ conflict('d', subopts, D_SW,
+ D_NOALIGN);
+ if (dswidth)
+ conflict('d', subopts, D_SWIDTH,
+ D_NOALIGN);
+ }
break;
case D_SECTLOG:
if (slflag)
@@ -1638,8 +1727,8 @@ main(
switch (getsubopt(&p, (constpp)subopts,
&value)) {
case I_ALIGN:
- sb_feat.inode_align = getbool(
- value, "i align", true);
+ sb_feat.inode_align = getnum_checked(
+ value, &iopts, I_ALIGN);
break;
case I_LOG:
if (ilflag)
@@ -1709,16 +1798,14 @@ main(
sb_feat.attr_version = c;
break;
case I_PROJID32BIT:
- sb_feat.projid16bit = !getbool(value,
- "i projid32bit", false);
+ sb_feat.projid16bit =
+ !getnum_checked(value, &iopts,
+ I_PROJID32BIT);
break;
case I_SPINODES:
- if (!value || *value == '\0')
- value = "1";
- c = atoi(value);
- if (c < 0 || c > 1)
- illegal(value, "i spinodes");
- sb_feat.spinodes = c;
+ sb_feat.spinodes =
+ getnum_checked(value, &iopts,
+ I_SPINODES);
break;
default:
unknown('i', value);
@@ -1748,11 +1835,11 @@ main(
laflag = 1;
break;
case L_FILE:
- if (loginternal)
+ xi.lisfile = getnum_checked(value,
+ &lopts, L_FILE);
+ if (xi.lisfile && loginternal)
conflict('l', subopts, L_INTERNAL,
L_FILE);
- xi.lisfile = getbool(value, "l file",
- true);
if (xi.lisfile)
xi.lcreat = 1;
break;
@@ -1765,8 +1852,8 @@ main(
if (liflag)
respec('l', subopts, L_INTERNAL);

- loginternal = getbool(value,
- "l internal", true);
+ loginternal = getnum_checked(value,
+ &lopts, L_INTERNAL);
liflag = 1;
break;
case L_SU:
@@ -1853,9 +1940,9 @@ main(
lssflag = 1;
break;
case L_LAZYSBCNTR:
- sb_feat.lazy_sb_counters = getbool(
- value, "l lazy-count",
- true);
+ sb_feat.lazy_sb_counters =
+ getnum_checked(value, &lopts,
+ L_LAZYSBCNTR);
break;
default:
unknown('l', value);
@@ -1876,8 +1963,9 @@ main(
switch (getsubopt(&p, (constpp)subopts,
&value)) {
case M_CRC:
- sb_feat.crcs_enabled = getbool(
- value, "m crc", true);
+ sb_feat.crcs_enabled =
+ getnum_checked(value, &mopts,
+ M_CRC);
if (sb_feat.crcs_enabled && nftype) {
fprintf(stderr,
_("cannot specify both -m crc=1 and -n ftype\n"));
@@ -1888,8 +1976,8 @@ _("cannot specify both -m crc=1 and -n ftype\n"));
break;
case M_FINOBT:
sb_feat.finobtflag = true;
- sb_feat.finobt = getbool(
- value, "m finobt", true);
+ sb_feat.finobt = getnum_checked(
+ value, &mopts, M_FINOBT);
break;
case M_UUID:
if (!value || *value == '\0')
@@ -1963,8 +2051,8 @@ _("cannot specify both -m crc=1 and -n ftype\n"));
_("cannot specify both -m crc=1 and -n ftype\n"));
usage();
}
- sb_feat.dirftype = getbool(value,
- "n ftype", true);
+ sb_feat.dirftype = getnum_checked(value,
+ &nopts, N_FTYPE);
nftype = 1;
break;
default:
@@ -2002,8 +2090,8 @@ _("cannot specify both -m crc=1 and -n ftype\n"));
rtextsize = value;
break;
case R_FILE:
- xi.risfile = getbool(value,
- "r file", true);
+ xi.risfile = getnum_checked(value,
+ &ropts, R_FILE);
if (xi.risfile)
xi.rcreat = 1;
break;
@@ -2023,7 +2111,8 @@ _("cannot specify both -m crc=1 and -n ftype\n"));
rtsize = value;
break;
case R_NOALIGN:
- norsflag = 1;
+ norsflag = getnum_checked(value,
+ &ropts, R_NOALIGN);
break;
default:
unknown('r', value);
--
2.6.0