[PATCH 14/17] mkfs: add string options to generic parsing

Jan Ťulák
2015-06-19T11:02:34Z

From: Dave Chinner <dchinner@redhat.com>

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@redhat.com>
Signed-off-by: Jan Ťulák <jtulak@redhat.com>
---
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