On Fri, Jun 19, 2015 at 01:02:03PM +0200, Jan Ťulák wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> So that string options are correctly detected for conflicts and
> respecification, add a getstr() function that modifies the option
> tables appropriately.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> Signed-off-by: Jan Ťulák <jtulak@xxxxxxxxxx>
> ---
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> mkfs/xfs_mkfs.c | 127 +++++++++++++++++++++++++++++++-------------------------
> 1 file changed, 70 insertions(+), 57 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 1d80188..6bfa73c 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -68,6 +68,7 @@ struct opt_params {
> struct subopt_param {
> int index;
> bool seen;
> + bool str_seen;
> bool convert;
> bool is_power_2;
> int conflicts[MAX_CONFLICTS];
> @@ -1412,14 +1413,15 @@ illegal_option(
> usage();
> }
>
> -static long long
> -getnum(
> +static void
> +check_opt(
> const char *str,
> struct opt_params *opts,
> - int index)
> + int index,
> + bool str_seen)
> {
> - struct subopt_param *sp = &opts->subopt_params[index];
> - long long c;
> + struct subopt_param *sp = &opts->subopt_params[index];
> + int i;
>
> if (sp->index != index) {
> fprintf(stderr,
> @@ -1428,21 +1430,46 @@ getnum(
> reqval(opts->name, (char **)opts->subopts, index);
> }
>
> - /* check for respecification of the option */
> - if (sp->seen)
> - respec(opts->name, (char **)opts->subopts, index);
> - sp->seen = true;
> + /*
> + * Check for respecification of the option. This is more complex than it
> + * seems because some options are parsed twice - once as a string during
> + * input parsing, then later the string is passed to getnum for
> + * conversion into a number and bounds checking. Hence the two variables
> + * used to track the different uses based on the @str parameter passed
> + * to us.
> + */
> + if (!str_seen) {
> + if (sp->seen)
> + respec(opts->name, (char **)opts->subopts, index);
> + sp->seen = true;
> + } else {
> + if (sp->str_seen)
> + respec(opts->name, (char **)opts->subopts, index);
> + sp->str_seen = true;
> + }
>
> /* check for conflicts with the option */
> - for (c = 0; c < MAX_CONFLICTS; c++) {
> - int conflict_opt = sp->conflicts[c];
> + for (i = 0; i < MAX_CONFLICTS; i++) {
> + int conflict_opt = sp->conflicts[i];
>
> if (conflict_opt == LAST_CONFLICT)
> break;
> - if (opts->subopt_params[conflict_opt].seen)
> + if (opts->subopt_params[conflict_opt].seen ||
> + opts->subopt_params[conflict_opt].str_seen)
> conflict(opts->name, (char **)opts->subopts, conflict_opt, index);
> }
> +}
> +
> +static long long
> +getnum(
> + const char *str,
> + struct opt_params *opts,
> + int index)
> +{
> + struct subopt_param *sp = &opts->subopt_params[index];
> + long long c;
>
> + check_opt(str, opts, index, false);
> /* empty strings might just return a default value */
> if (!str || *str == '\0') {
> if (sp->defaultval == SUBOPT_NEEDS_VAL)
> @@ -1476,6 +1503,26 @@ getnum(
> return c;
> }
>
> +/*
> + * Option is a string - do all the option table work, and check there
> + * is actually an option string. Otherwise we don't do anything with the string
> + * here - validation will be done later when the string is converted to a value
> + * or used as a file/device path.
> + */
> +static char *
> +getstr(
> + char *str,
> + struct opt_params *opts,
> + int index)
> +{
> + check_opt(str, opts, index, true);
> +
> + /* empty strings for string options are not valid */
> + if (!str || *str == '\0')
> + reqval(opts->name, (char **)opts->subopts, index);
> + return str;
> +}
> +
> int
> main(
> int argc,
> @@ -1670,18 +1717,10 @@ main(
> xi.dcreat = 1;
> break;
> case D_NAME:
> - if (!value || *value == '\0')
> - reqval('d', subopts, D_NAME);
> - if (xi.dname)
> - respec('d', subopts, D_NAME);
> - xi.dname = value;
> + xi.dname = getstr(value, &dopts, D_NAME);
> break;
> case D_SIZE:
> - if (!value || *value == '\0')
> - reqval('d', subopts, D_SIZE);
> - if (dsize)
> - respec('d', subopts, D_SIZE);
> - dsize = value;
> + dsize = getstr(value, &dopts, D_SIZE);
> break;
> case D_SUNIT:
> dsunit = getnum(value, &dopts, D_SUNIT);
> @@ -1822,18 +1861,10 @@ main(
> break;
> case L_NAME:
> case L_DEV:
> - if (laflag)
> - conflict('l', subopts, L_AGNUM, L_DEV);
> - if (liflag)
> - conflict('l', subopts, L_INTERNAL, L_DEV);
> - if (!value || *value == '\0')
> - reqval('l', subopts, L_NAME);
> - if (xi.logname)
> - respec('l', subopts, L_NAME);
> + logfile = getstr(value, &lopts, L_NAME);
> + xi.logname = logfile;
> ldflag = 1;
> loginternal = 0;
> - logfile = value;
> - xi.logname = value;
> break;
> case L_VERSION:
> sb_feat.log_version =
> @@ -1841,12 +1872,7 @@ main(
> lvflag = 1;
> break;
> case L_SIZE:
> - if (!value || *value == '\0')
> - reqval('l', subopts, L_SIZE);
> - if (logsize)
> - respec('l', subopts, L_SIZE);
> - logsize = value;
> - lsflag = 1;
> + logsize = getstr(value, &lopts, L_SIZE);
> break;
> case L_SECTLOG:
> lsectorlog = getnum(value, &lopts,
> @@ -1930,10 +1956,7 @@ _("cannot specify both -m crc=1 and -n ftype\n"));
> nsflag = 1;
> break;
> case N_VERSION:
> - if (!value || *value == '\0')
> - reqval('n', subopts, N_VERSION);
> - if (nvflag)
> - respec('n', subopts, N_VERSION);
> + value = getstr(value, &nopts, N_VERSION);
> if (!strcasecmp(value, "ci")) {
> /* ASCII CI mode */
> sb_feat.nci = true;
> @@ -1982,11 +2005,8 @@ _("cannot specify both -m crc=1 and -n ftype\n"));
> switch (getsubopt(&p, (constpp)subopts,
> &value)) {
> case R_EXTSIZE:
> - if (!value || *value == '\0')
> - reqval('r', subopts, R_EXTSIZE);
> - if (rtextsize)
> - respec('r', subopts, R_EXTSIZE);
> - rtextsize = value;
> + rtextsize = getstr(value, &ropts,
> + R_EXTSIZE);
> break;
> case R_FILE:
> xi.risfile = getnum(value, &ropts,
> @@ -1996,18 +2016,11 @@ _("cannot specify both -m crc=1 and -n ftype\n"));
> break;
> case R_NAME:
> case R_DEV:
> - if (!value || *value == '\0')
> - reqval('r', subopts, R_NAME);
> - if (xi.rtname)
> - respec('r', subopts, R_NAME);
> - xi.rtname = value;
> + xi.rtname = getstr(value, &ropts,
> + R_NAME);
> break;
> case R_SIZE:
> - if (!value || *value == '\0')
> - reqval('r', subopts, R_SIZE);
> - if (rtsize)
> - respec('r', subopts, R_SIZE);
> - rtsize = value;
> + rtsize = getstr(value, &ropts, R_SIZE);
> break;
> case R_NOALIGN:
> norsflag = 1;
> --
> 2.1.0
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
