On Fri, Jun 19, 2015 at 01:01:57PM +0200, Jan ÅulÃk wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> getbool() can be replaced with getnum_checked with appropriate
> min/max values set for the boolean variables.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> Signed-off-by: Jan ÅulÃk <jtulak@xxxxxxxxxx>
> ---
> mkfs/xfs_mkfs.c | 147
> ++++++++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 106 insertions(+), 41 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index d3d1e11..3803779 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -53,6 +53,7 @@ static long long cvtnum(unsigned int blocksize,
> unsigned int sectorsize, const char *s);
>
> #define MAX_SUBOPTS 16
> +#define SUBOPT_NEEDS_VAL (-1LL)
> struct opt_params {
> const char name;
> const char *subopts[MAX_SUBOPTS];
> @@ -60,6 +61,7 @@ struct opt_params {
> int index;
> long long minval;
> long long maxval;
> + long long defaultval;
> } subopt_params[MAX_SUBOPTS];
> };
>
> @@ -76,10 +78,12 @@ const 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,
> },
> },
> };
> @@ -121,38 +125,55 @@ const 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,
> + .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = D_RTINHERIT,
> + .defaultval = SUBOPT_NEEDS_VAL,
> },
I don't think the above two require values.
> { .index = D_PROJINHERIT,
> + .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = D_EXTSZINHERIT,
> + .defaultval = SUBOPT_NEEDS_VAL,
> },
> },
> };
> @@ -181,20 +202,31 @@ const 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,
Need to handle sparse here too once it's added to the previous patch.
> },
> },
> };
> @@ -230,32 +262,50 @@ const 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,
> },
> },
> };
> @@ -277,14 +327,20 @@ const 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 = 0,
> },
> },
> };
> @@ -308,16 +364,22 @@ const 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,
> + .defaultval = SUBOPT_NEEDS_VAL,
This should have min/max/default similar to D_FILE, yes?
> },
> { .index = R_NAME,
> + .defaultval = SUBOPT_NEEDS_VAL,
> },
> { .index = R_NOALIGN,
> + .defaultval = SUBOPT_NEEDS_VAL,
Not sure about this one, either. There's no value according to the man
page.
> },
> },
> };
> @@ -339,18 +401,22 @@ const 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,
> },
> },
> };
> @@ -366,6 +432,9 @@ const struct opt_params mopts = {
> },
> .subopt_params = {
> { .index = M_CRC,
> + .minval = 0,
> + .maxval = 1,
> + .defaultval = 0,
> },
> },
How about M_FINOBT?
Brian
> };
> @@ -1205,22 +1274,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,
> @@ -1235,18 +1288,28 @@ illegal_option(
>
> static int
> getnum_checked(
> - const char *str,
> + const char *str,
> const 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 (!str || *str == '\0') {
> + if (sp->defaultval == SUBOPT_NEEDS_VAL)
> + reqval(opts->name, (char **)opts->subopts, index);
> + return sp->defaultval;
> + }
>
> 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;
> }
> @@ -1470,8 +1533,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;
> @@ -1613,8 +1676,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)
> @@ -1684,8 +1747,9 @@ 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')
> @@ -1723,8 +1787,8 @@ main(
> if (loginternal)
> conflict('l', subopts,
> L_INTERNAL,
> L_FILE);
> - xi.lisfile = getbool(value, "l file",
> - true);
> + xi.lisfile = getnum_checked(value,
> + &lopts, L_FILE);
> if (xi.lisfile)
> xi.lcreat = 1;
> break;
> @@ -1737,8 +1801,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:
> @@ -1825,9 +1889,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);
> @@ -1848,8 +1912,9 @@ main(
> switch (getsubopt(&p, (constpp)subopts,
> &value)) {
> case M_CRC:
> - sb_feat.crcs_enabled = getbool(
> - value, "m crc", false);
> + 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"));
> @@ -1932,8 +1997,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", false);
> + sb_feat.dirftype = getnum_checked(value,
> + &nopts,
> N_FTYPE);
> nftype = 1;
> break;
> default:
> @@ -1971,8 +2036,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;
> --
> 2.1.0
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
|