Pass consistent param->type to fs_parse()

As it is, vfs_parse_fs_string() makes "foo" and "foo=" indistinguishable;
both get fs_value_is_string for ->type and NULL for ->string.  To make
it even more unpleasant, that combination is impossible to produce with
fsconfig().

Much saner rules would be
        "foo"           => fs_value_is_flag, NULL
	"foo="          => fs_value_is_string, ""
	"foo=bar"       => fs_value_is_string, "bar"
All cases are distinguishable, all results are expressable by fsconfig(),
->has_value checks are much simpler that way (to the point of the field
being useless) and quite a few regressions go away (gfs2 has no business
accepting -o nodebug=, for example).

Partially based upon patches from Miklos.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 2b184563cd32e8ca6c45ffbad4dcad1626429194..9fc686be81caa7252ef19b168656744530c8478c 100644 (file)
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -6433,7 +6433,7 @@ static int rbd_parse_options(char *options, struct rbd_parse_opts_ctx *pctx)
                if (*key) {
                        struct fs_parameter param = {
                                .key    = key,
-                               .type   = fs_value_is_string,
+                               .type   = fs_value_is_flag,
                        };
                        char *value = strchr(key, '=');
                        size_t v_len = 0;
@@ -6443,14 +6443,11 @@ static int rbd_parse_options(char *options, struct rbd_parse_opts_ctx *pctx)
                                        continue;
                                *value++ = 0;
                                v_len = strlen(value);
-                       }
-
-
-                       if (v_len > 0) {
                                param.string = kmemdup_nul(value, v_len,
                                                           GFP_KERNEL);
                                if (!param.string)
                                        return -ENOMEM;
+                               param.type = fs_value_is_string;
                        }
                        param.size = v_len;
 

diff --git a/fs/fs_context.c b/fs/fs_context.c
index 138b5b4d621d2e86527a2116a2688b07eb11d3cb..9097421cbba5571b3ca0b78e984a05d5bd5be2ae 100644 (file)
--- a/fs/fs_context.c
+++ b/fs/fs_context.c
@@ -175,14 +175,15 @@ int vfs_parse_fs_string(struct fs_context *fc, const char *key,
 
        struct fs_parameter param = {
                .key    = key,
-               .type   = fs_value_is_string,
+               .type   = fs_value_is_flag,
                .size   = v_size,
        };
 
-       if (v_size > 0) {
+       if (value) {
                param.string = kmemdup_nul(value, v_size, GFP_KERNEL);
                if (!param.string)
                        return -ENOMEM;
+               param.type = fs_value_is_string;
        }
 
        ret = vfs_parse_fs_param(fc, &param);

diff --git a/fs/fs_parser.c b/fs/fs_parser.c
index d1930adce68da1fb226b894e15951e7dd895aa6c..07ae041b83a8bc918e10dd619d4c5b8812a62167 100644 (file)
--- a/fs/fs_parser.c
+++ b/fs/fs_parser.c
@@ -85,7 +85,6 @@ int fs_parse(struct fs_context *fc,
        const struct fs_parameter_enum *e;
        int ret = -ENOPARAM, b;
 
-       result->has_value = !!param->string;
        result->negated = false;
        result->uint_64 = 0;
 
@@ -95,7 +94,7 @@ int fs_parse(struct fs_context *fc,
                 * "xxx" takes the "no"-form negative - but only if there
                 * wasn't an value.
                 */
-               if (result->has_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;
@@ -127,14 +126,18 @@ int fs_parse(struct fs_context *fc,
        case fs_param_is_u64:
        case fs_param_is_enum:
        case fs_param_is_string:
-               if (param->type != fs_value_is_string)
-                       goto bad_value;
-               if (!result->has_value) {
+               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;
                }
-               /* Fall through */
+               goto bad_value;
        default:
                break;
        }
@@ -144,8 +147,7 @@ int fs_parse(struct fs_context *fc,
         */
        switch (p->type) {
        case fs_param_is_flag:
-               if (param->type != fs_value_is_flag &&
-                   (param->type != fs_value_is_string || result->has_value))
+               if (param->type != fs_value_is_flag)
                        return invalf(fc, "%s: Unexpected value for '%s'",
                                      desc->name, param->key);
                result->boolean = true;
@@ -206,9 +208,6 @@ int fs_parse(struct fs_context *fc,
        case fs_param_is_fd: {
                switch (param->type) {
                case fs_value_is_string:
-                       if (!result->has_value)
-                               goto bad_value;
-
                        ret = kstrtouint(param->string, 0, &result->uint_32);
                        break;
                case fs_value_is_file:

diff --git a/include/linux/fs_parser.h b/include/linux/fs_parser.h
index dee140db6240a7f8dfb21b7a2ca38ac11c9010e3..45323203128b9873589fea6d912f151a00f68637 100644 (file)
--- a/include/linux/fs_parser.h
+++ b/include/linux/fs_parser.h
@@ -72,7 +72,6 @@ struct fs_parameter_description {
  */
 struct fs_parse_result {
        bool                    negated;        /* T if param was "noxxx" */
-       bool                    has_value;      /* T if value supplied to param */
        union {
                bool            boolean;        /* For spec_bool */
                int             int_32;         /* For spec_s32/spec_enum */