From 48ce73b1bef20331007b35de7ade8fe26cd55e84 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 17 Dec 2019 20:03:59 -0500 Subject: fs_parse: handle optional arguments sanely 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 --- fs/ceph/super.c | 4 +- fs/fs_parser.c | 142 +++++++++++++++++++++------------------------------ fs/gfs2/ops_fstype.c | 38 ++++---------- fs/nfs/fs_context.c | 18 ++++--- 4 files changed, 82 insertions(+), 120 deletions(-) (limited to 'fs') diff --git a/fs/ceph/super.c b/fs/ceph/super.c index d52eb3edb45d..acdcf96b5bc7 100644 --- a/fs/ceph/super.c +++ b/fs/ceph/super.c @@ -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), diff --git a/fs/fs_parser.c b/fs/fs_parser.c index 5f8c06a1fb93..db940fac84c3 100644 --- a/fs/fs_parser.c +++ b/fs/fs_parser.c @@ -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; diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c index 32623d28612b..6d5502fff15c 100644 --- a/fs/gfs2/ops_fstype.c +++ b/fs/gfs2/ops_fstype.c @@ -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; diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c index 39f980a0ee48..c87cdedbdd0c 100644 --- a/fs/nfs/fs_context.c +++ b/fs/nfs/fs_context.c @@ -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) -- cgit