xfs
[Top] [All Lists]

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

To: xfs@xxxxxxxxxxx
Subject: [PATCH 14/17] mkfs: add string options to generic parsing
From: Jan ÅulÃk <jtulak@xxxxxxxxxx>
Date: Fri, 19 Jun 2015 13:02:03 +0200
Cc: david@xxxxxxxxxxxxx, Dave Chinner <dchinner@xxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1434711726-13092-1-git-send-email-jtulak@xxxxxxxxxx>
References: <1434711726-13092-1-git-send-email-jtulak@xxxxxxxxxx>
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>
---
 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

<Prev in Thread] Current Thread [Next in Thread>