fs_parse: handle optional arguments sanely
authorAl Viro <viro@zeniv.linux.org.uk>
Wed, 18 Dec 2019 01:03:59 +0000 (20:03 -0500)
committerAl Viro <viro@zeniv.linux.org.uk>
Fri, 7 Feb 2020 19:48:37 +0000 (14:48 -0500)
Don't bother with "mixed" options that would allow both the
form with and without argument (i.e. both -o foo and -o foo=bar).
Rather than trying to shove both into a single fs_parameter_spec,
allow having with-argument and no-argument specs with the same
name and teach fs_parse to handle that.

There are very few options of that sort, and they are actually
easier to handle that way - callers end up with less postprocessing.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
fs/ceph/super.c
fs/fs_parser.c
fs/gfs2/ops_fstype.c
fs/nfs/fs_context.c
include/linux/fs_parser.h

index d52eb3edb45d23446b842cb50dbdf962c12dc7ad..acdcf96b5bc7f8c6249a4fb68caeb5819fadd3ed 100644 (file)
@@ -179,8 +179,8 @@ static const struct fs_parameter_spec ceph_mount_parameters[] = {
        fsparam_flag_no ("copyfrom",                    Opt_copyfrom),
        fsparam_flag_no ("dcache",                      Opt_dcache),
        fsparam_flag_no ("dirstat",                     Opt_dirstat),
-       __fsparam       (fs_param_is_string, "fsc",     Opt_fscache,
-                        fs_param_neg_with_no | fs_param_v_optional, NULL),
+       fsparam_flag_no ("fsc",                         Opt_fscache), // fsc|nofsc
+       fsparam_string  ("fsc",                         Opt_fscache), // fsc=...
        fsparam_flag_no ("ino32",                       Opt_ino32),
        fsparam_string  ("mds_namespace",               Opt_mds_namespace),
        fsparam_flag_no ("poolperm",                    Opt_poolperm),
index 5f8c06a1fb93e4bbb415a3742941b21de44c892e..db940fac84c32ee653d29dec3f08976c25568c48 100644 (file)
@@ -46,19 +46,40 @@ int lookup_constant(const struct constant_table *tbl, const char *name, int not_
 }
 EXPORT_SYMBOL(lookup_constant);
 
+static inline bool is_flag(const struct fs_parameter_spec *p)
+{
+       return p->type == fs_param_is_flag;
+}
+
 static const struct fs_parameter_spec *fs_lookup_key(
        const struct fs_parameter_spec *desc,
-       const char *name)
+       struct fs_parameter *param, bool *negated)
 {
-       const struct fs_parameter_spec *p;
-       if (!desc)
-               return NULL;
-
-       for (p = desc; p->name; p++)
-               if (strcmp(p->name, name) == 0)
+       const struct fs_parameter_spec *p, *other = NULL;
+       const char *name = param->key;
+       bool want_flag = param->type == fs_value_is_flag;
+
+       *negated = false;
+       for (p = desc; p->name; p++) {
+               if (strcmp(p->name, name) != 0)
+                       continue;
+               if (likely(is_flag(p) == want_flag))
                        return p;
-
-       return NULL;
+               other = p;
+       }
+       if (want_flag) {
+               if (name[0] == 'n' && name[1] == 'o' && name[2]) {
+                       for (p = desc; p->name; p++) {
+                               if (strcmp(p->name, name + 2) != 0)
+                                       continue;
+                               if (!(p->flags & fs_param_neg_with_no))
+                                       continue;
+                               *negated = true;
+                               return p;
+                       }
+               }
+       }
+       return other;
 }
 
 /*
@@ -88,123 +109,77 @@ int __fs_parse(struct p_log *log,
        const struct constant_table *e;
        int ret = -ENOPARAM, b;
 
-       result->negated = false;
        result->uint_64 = 0;
 
-       p = fs_lookup_key(desc, param->key);
-       if (!p) {
-               /* If we didn't find something that looks like "noxxx", see if
-                * "xxx" takes the "no"-form negative - but only if there
-                * wasn't an value.
-                */
-               if (param->type != fs_value_is_flag)
-                       goto unknown_parameter;
-               if (param->key[0] != 'n' || param->key[1] != 'o' || !param->key[2])
-                       goto unknown_parameter;
-
-               p = fs_lookup_key(desc, param->key + 2);
-               if (!p)
-                       goto unknown_parameter;
-               if (!(p->flags & fs_param_neg_with_no))
-                       goto unknown_parameter;
-               result->boolean = false;
-               result->negated = true;
-       }
+       p = fs_lookup_key(desc, param, &result->negated);
+       if (!p)
+               return -ENOPARAM;
 
        if (p->flags & fs_param_deprecated)
                warn_plog(log, "Deprecated parameter '%s'", param->key);
 
-       if (result->negated)
-               goto okay;
-
-       /* Certain parameter types only take a string and convert it. */
-       switch (p->type) {
-       case __fs_param_wasnt_defined:
-               return -EINVAL;
-       case fs_param_is_u32:
-       case fs_param_is_u32_octal:
-       case fs_param_is_u32_hex:
-       case fs_param_is_s32:
-       case fs_param_is_u64:
-       case fs_param_is_enum:
-       case fs_param_is_string:
-               if (param->type == fs_value_is_string) {
-                       if (p->flags & fs_param_v_optional)
-                               break;
-                       if (!*param->string)
-                               goto bad_value;
-                       break;
-               }
-               if (param->type == fs_value_is_flag) {
-                       if (p->flags & fs_param_v_optional)
-                               goto okay;
-               }
-               goto bad_value;
-       default:
-               break;
-       }
-
        /* Try to turn the type we were given into the type desired by the
         * parameter and give an error if we can't.
         */
        switch (p->type) {
+       case __fs_param_wasnt_defined:
+               return -EINVAL;
        case fs_param_is_flag:
                if (param->type != fs_value_is_flag)
                        return inval_plog(log, "Unexpected value for '%s'",
                                      param->key);
-               result->boolean = true;
+               result->boolean = !result->negated;
                goto okay;
-
        case fs_param_is_bool:
-               switch (param->type) {
-               case fs_value_is_flag:
-                       result->boolean = true;
-                       goto okay;
-               case fs_value_is_string:
-                       if (param->size == 0) {
-                               result->boolean = true;
-                               goto okay;
-                       }
-                       b = lookup_constant(bool_names, param->string, -1);
-                       if (b == -1)
-                               goto bad_value;
-                       result->boolean = b;
-                       goto okay;
-               default:
+               if (param->type != fs_value_is_string)
                        goto bad_value;
-               }
-
+               b = lookup_constant(bool_names, param->string, -1);
+               if (b == -1)
+                       goto bad_value;
+               result->boolean = b;
+               goto okay;
        case fs_param_is_u32:
+               if (param->type != fs_value_is_string)
+                       goto bad_value;
                ret = kstrtouint(param->string, 0, &result->uint_32);
                goto maybe_okay;
        case fs_param_is_u32_octal:
+               if (param->type != fs_value_is_string)
+                       goto bad_value;
                ret = kstrtouint(param->string, 8, &result->uint_32);
                goto maybe_okay;
        case fs_param_is_u32_hex:
+               if (param->type != fs_value_is_string)
+                       goto bad_value;
                ret = kstrtouint(param->string, 16, &result->uint_32);
                goto maybe_okay;
        case fs_param_is_s32:
+               if (param->type != fs_value_is_string)
+                       goto bad_value;
                ret = kstrtoint(param->string, 0, &result->int_32);
                goto maybe_okay;
        case fs_param_is_u64:
+               if (param->type != fs_value_is_string)
+                       goto bad_value;
                ret = kstrtoull(param->string, 0, &result->uint_64);
                goto maybe_okay;
-
        case fs_param_is_enum:
+               if (param->type != fs_value_is_string)
+                       goto bad_value;
                e = __lookup_constant(p->data, param->string);
                if (e) {
                        result->uint_32 = e->value;
                        goto okay;
                }
                goto bad_value;
-
        case fs_param_is_string:
+               if (param->type != fs_value_is_string || !*param->string)
+                       goto bad_value;
                goto okay;
        case fs_param_is_blob:
                if (param->type != fs_value_is_blob)
                        goto bad_value;
                goto okay;
-
        case fs_param_is_fd: {
                switch (param->type) {
                case fs_value_is_string:
@@ -221,7 +196,6 @@ int __fs_parse(struct p_log *log,
                        goto bad_value;
                goto maybe_okay;
        }
-
        case fs_param_is_blockdev:
        case fs_param_is_path:
                goto okay;
@@ -237,8 +211,6 @@ okay:
 
 bad_value:
        return inval_plog(log, "Bad value for '%s'", param->key);
-unknown_parameter:
-       return -ENOPARAM;
 }
 EXPORT_SYMBOL(__fs_parse);
 
@@ -382,6 +354,8 @@ bool fs_validate_description(const char *name,
                /* Check for duplicate parameter names */
                for (p2 = desc; p2 < param; p2++) {
                        if (strcmp(param->name, p2->name) == 0) {
+                               if (is_flag(param) != is_flag(p2))
+                                       continue;
                                pr_err("VALIDATE %s: PARAM[%s]: Duplicate\n",
                                       name, param->name);
                                good = false;
index 32623d28612bf9e025bd6120e91f06b44943ac1d..6d5502fff15cc169efea9d55a32a9a2fd2ce47ba 100644 (file)
@@ -1250,6 +1250,7 @@ enum gfs2_param {
        Opt_upgrade,
        Opt_acl,
        Opt_quota,
+       Opt_quota_flag,
        Opt_suiddir,
        Opt_data,
        Opt_meta,
@@ -1264,26 +1265,13 @@ enum gfs2_param {
        Opt_loccookie,
 };
 
-enum opt_quota {
-       Opt_quota_unset = 0,
-       Opt_quota_off,
-       Opt_quota_account,
-       Opt_quota_on,
-};
-
 static const struct constant_table gfs2_param_quota[] = {
-       {"off",        Opt_quota_off },
-       {"account",    Opt_quota_account },
-       {"on",         Opt_quota_on },
+       {"off",        GFS2_QUOTA_OFF},
+       {"account",    GFS2_QUOTA_ACCOUNT},
+       {"on",         GFS2_QUOTA_ON},
        {}
 };
 
-static const unsigned int opt_quota_values[] = {
-       [Opt_quota_off]     = GFS2_QUOTA_OFF,
-       [Opt_quota_account] = GFS2_QUOTA_ACCOUNT,
-       [Opt_quota_on]      = GFS2_QUOTA_ON,
-};
-
 enum opt_data {
        Opt_data_writeback = GFS2_DATA_WRITEBACK,
        Opt_data_ordered   = GFS2_DATA_ORDERED,
@@ -1331,8 +1319,8 @@ static const struct fs_parameter_spec gfs2_fs_parameters[] = {
        fsparam_flag_no("rgrplvb",            Opt_rgrplvb),
        fsparam_flag_no("loccookie",          Opt_loccookie),
        /* quota can be a flag or an enum so it gets special treatment */
-       __fsparam(fs_param_is_enum, "quota", Opt_quota,
-               fs_param_neg_with_no|fs_param_v_optional, gfs2_param_quota),
+       fsparam_flag_no("quota",              Opt_quota_flag),
+       fsparam_enum("quota",                 Opt_quota, gfs2_param_quota),
        {}
 };
 
@@ -1380,17 +1368,11 @@ static int gfs2_parse_param(struct fs_context *fc, struct fs_parameter *param)
        case Opt_acl:
                args->ar_posix_acl = result.boolean;
                break;
+       case Opt_quota_flag:
+               args->ar_quota = result.negated ? GFS2_QUOTA_OFF : GFS2_QUOTA_ON;
+               break;
        case Opt_quota:
-               /* The quota option can be a flag or an enum. A non-zero int_32
-                  result means that we have an enum index. Otherwise we have
-                  to rely on the 'negated' flag to tell us whether 'quota' or
-                  'noquota' was specified. */
-               if (result.negated)
-                       args->ar_quota = GFS2_QUOTA_OFF;
-               else if (result.int_32 > 0)
-                       args->ar_quota = opt_quota_values[result.int_32];
-               else
-                       args->ar_quota = GFS2_QUOTA_ON;
+               args->ar_quota = result.int_32;
                break;
        case Opt_suiddir:
                args->ar_suiddir = result.boolean;
index 39f980a0ee48abfe30acca57490b6e3180905914..c87cdedbdd0c4ecc3a4575e1c777e5a93840954e 100644 (file)
@@ -45,6 +45,7 @@ enum nfs_param {
        Opt_cto,
        Opt_fg,
        Opt_fscache,
+       Opt_fscache_flag,
        Opt_hard,
        Opt_intr,
        Opt_local_lock,
@@ -125,8 +126,8 @@ static const struct fs_parameter_spec nfs_fs_parameters[] = {
        fsparam_string("clientaddr",    Opt_clientaddr),
        fsparam_flag_no("cto",          Opt_cto),
        fsparam_flag  ("fg",            Opt_fg),
-       __fsparam(fs_param_is_string, "fsc",            Opt_fscache,
-                 fs_param_neg_with_no|fs_param_v_optional, NULL),
+       fsparam_flag_no("fsc",          Opt_fscache_flag),
+       fsparam_string("fsc",           Opt_fscache),
        fsparam_flag  ("hard",          Opt_hard),
        __fsparam(fs_param_is_flag, "intr",             Opt_intr,
                  fs_param_neg_with_no|fs_param_deprecated, NULL),
@@ -537,14 +538,19 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
                else
                        ctx->flags &= ~NFS_MOUNT_NORESVPORT;
                break;
-       case Opt_fscache:
-               kfree(ctx->fscache_uniq);
-               ctx->fscache_uniq = param->string;
-               param->string = NULL;
+       case Opt_fscache_flag:
                if (result.negated)
                        ctx->options &= ~NFS_OPTION_FSCACHE;
                else
                        ctx->options |= NFS_OPTION_FSCACHE;
+               kfree(ctx->fscache_uniq);
+               ctx->fscache_uniq = NULL;
+               break;
+       case Opt_fscache:
+               ctx->options |= NFS_OPTION_FSCACHE;
+               kfree(ctx->fscache_uniq);
+               ctx->fscache_uniq = param->string;
+               param->string = NULL;
                break;
        case Opt_migration:
                if (result.negated)
index dcbac245e7a3dee66529459aeba1fb48dcb5067c..2e1e15f0cf4ac6bd5f08a54a1ff5f3e409b2ef24 100644 (file)
@@ -49,7 +49,6 @@ struct fs_parameter_spec {
        u8                      opt;    /* Option number (returned by fs_parse()) */
        enum fs_parameter_type  type:8; /* The desired parameter type */
        unsigned short          flags;
-#define fs_param_v_optional    0x0001  /* The value is optional */
 #define fs_param_neg_with_no   0x0002  /* "noxxx" is negative param */
 #define fs_param_neg_with_empty        0x0004  /* "xxx=" is negative param */
 #define fs_param_deprecated    0x0008  /* The param is deprecated */