* [PATCH] fs: fs_parser: remove fs_parameter_description name field
@ 2019-08-27 0:46 Eric Sandeen
2019-09-06 9:17 ` David Howells
2019-09-06 13:01 ` David Howells
0 siblings, 2 replies; 5+ messages in thread
From: Eric Sandeen @ 2019-08-27 0:46 UTC (permalink / raw)
To: fsdevel; +Cc: David Howells
There doesn't seem to be a strong reason to have another copy of the
filesystem name string in the fs_parameter_description structure;
it's easy enough to get the name from the fs_type, and using it
instead ensures consistency across messages (for example,
vfs_parse_fs_param() already uses fc->fs_type->name for the error
messages, because it doesn't have the fs_parameter_description).
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
If I'm missing a reason for the separate copy of the string, feel
free to nak...
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index a46dee8e78db..5999eae23308 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -2047,7 +2047,6 @@ static const struct fs_parameter_spec rdt_param_specs[] = {
};
static const struct fs_parameter_description rdt_fs_parameters = {
- .name = "rdt",
.specs = rdt_param_specs,
};
diff --git a/fs/afs/super.c b/fs/afs/super.c
index f18911e8d770..b7c2dd4219dd 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -90,7 +90,6 @@ static const struct fs_parameter_enum afs_param_enums[] = {
};
static const struct fs_parameter_description afs_fs_parameters = {
- .name = "kAFS",
.specs = afs_param_specs,
.enums = afs_param_enums,
};
diff --git a/fs/filesystems.c b/fs/filesystems.c
index 9135646e41ac..77bf5f95362d 100644
--- a/fs/filesystems.c
+++ b/fs/filesystems.c
@@ -74,7 +74,8 @@ int register_filesystem(struct file_system_type * fs)
int res = 0;
struct file_system_type ** p;
- if (fs->parameters && !fs_validate_description(fs->parameters))
+ if (fs->parameters &&
+ !fs_validate_description(fs->name, fs->parameters))
return -EINVAL;
BUG_ON(strchr(fs->name, '.'));
diff --git a/fs/fs_parser.c b/fs/fs_parser.c
index 460ea4206fa2..43d5ca08e629 100644
--- a/fs/fs_parser.c
+++ b/fs/fs_parser.c
@@ -111,7 +111,7 @@ int fs_parse(struct fs_context *fc,
if (p->flags & fs_param_deprecated)
warnf(fc, "%s: Deprecated parameter '%s'",
- desc->name, param->key);
+ fc->fs_type->name, param->key);
if (result->negated)
goto okay;
@@ -147,7 +147,7 @@ int fs_parse(struct fs_context *fc,
if (param->type != fs_value_is_flag &&
(param->type != fs_value_is_string || result->has_value))
return invalf(fc, "%s: Unexpected value for '%s'",
- desc->name, param->key);
+ fc->fs_type->name, param->key);
result->boolean = true;
goto okay;
@@ -223,7 +223,8 @@ int fs_parse(struct fs_context *fc,
return p->opt;
bad_value:
- return invalf(fc, "%s: Bad value for '%s'", desc->name, param->key);
+ return invalf(fc, "%s: Bad value for '%s'",
+ fc->fs_type->name, param->key);
unknown_parameter:
return -ENOPARAM;
}
@@ -343,22 +344,16 @@ bool validate_constant_table(const struct constant_table *tbl, size_t tbl_size,
* fs_validate_description - Validate a parameter description
* @desc: The parameter description to validate.
*/
-bool fs_validate_description(const struct fs_parameter_description *desc)
+bool fs_validate_description(const char *name,
+ const struct fs_parameter_description *desc)
{
const struct fs_parameter_spec *param, *p2;
const struct fs_parameter_enum *e;
- const char *name = desc->name;
unsigned int nr_params = 0;
bool good = true, enums = false;
pr_notice("*** VALIDATE %s ***\n", name);
- if (!name[0]) {
- pr_err("VALIDATE Parser: No name\n");
- name = "Unknown";
- good = false;
- }
-
if (desc->specs) {
for (param = desc->specs; param->name; param++) {
enum fs_parameter_type t = param->type;
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index a478df035651..67b76e0125a4 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -85,7 +85,6 @@ static const struct fs_parameter_spec hugetlb_param_specs[] = {
};
static const struct fs_parameter_description hugetlb_fs_parameters = {
- .name = "hugetlbfs",
.specs = hugetlb_param_specs,
};
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 33f72d1b92cc..b3dc8e0da732 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -48,7 +48,6 @@ static const struct fs_parameter_spec proc_param_specs[] = {
};
static const struct fs_parameter_description proc_fs_parameters = {
- .name = "proc",
.specs = proc_param_specs,
};
diff --git a/include/linux/fs_parser.h b/include/linux/fs_parser.h
index dee140db6240..090a2edf3e72 100644
--- a/include/linux/fs_parser.h
+++ b/include/linux/fs_parser.h
@@ -62,7 +62,6 @@ struct fs_parameter_enum {
};
struct fs_parameter_description {
- const char name[16]; /* Name for logging purposes */
const struct fs_parameter_spec *specs; /* List of param specifications */
const struct fs_parameter_enum *enums; /* Enum values */
};
@@ -97,12 +96,14 @@ extern int __lookup_constant(const struct constant_table tbl[], size_t tbl_size,
#ifdef CONFIG_VALIDATE_FS_PARSER
extern bool validate_constant_table(const struct constant_table *tbl, size_t tbl_size,
int low, int high, int special);
-extern bool fs_validate_description(const struct fs_parameter_description *desc);
+extern bool fs_validate_description(const char *name,
+ const struct fs_parameter_description *desc);
#else
static inline bool validate_constant_table(const struct constant_table *tbl, size_t tbl_size,
int low, int high, int special)
{ return true; }
-static inline bool fs_validate_description(const struct fs_parameter_description *desc)
+static inline bool fs_validate_description(const char *name,
+ const struct fs_parameter_description *desc)
{ return true; }
#endif
diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
index 88006be40ea3..9a317f08b323 100644
--- a/kernel/cgroup/cgroup-v1.c
+++ b/kernel/cgroup/cgroup-v1.c
@@ -918,7 +918,6 @@ static const struct fs_parameter_spec cgroup1_param_specs[] = {
};
const struct fs_parameter_description cgroup1_fs_parameters = {
- .name = "cgroup1",
.specs = cgroup1_param_specs,
};
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 753afbca549f..ee3a84aa6599 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1826,7 +1826,6 @@ static const struct fs_parameter_spec cgroup2_param_specs[] = {
};
static const struct fs_parameter_description cgroup2_fs_parameters = {
- .name = "cgroup2",
.specs = cgroup2_param_specs,
};
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 74dd46de01b6..9e45cf94cd4d 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2820,7 +2820,6 @@ static const struct fs_parameter_spec selinux_param_specs[] = {
};
static const struct fs_parameter_description selinux_fs_parameters = {
- .name = "SELinux",
.specs = selinux_param_specs,
};
@@ -7021,7 +7020,7 @@ static __init int selinux_init(void)
else
pr_debug("SELinux: Starting in permissive mode\n");
- fs_validate_description(&selinux_fs_parameters);
+ fs_validate_description("selinux", &selinux_fs_parameters);
return 0;
}
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 4c5e5a438f8b..87d9e4a1e3ed 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -690,7 +690,6 @@ static const struct fs_parameter_spec smack_param_specs[] = {
};
static const struct fs_parameter_description smack_fs_parameters = {
- .name = "smack",
.specs = smack_param_specs,
};
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] fs: fs_parser: remove fs_parameter_description name field
2019-08-27 0:46 [PATCH] fs: fs_parser: remove fs_parameter_description name field Eric Sandeen
@ 2019-09-06 9:17 ` David Howells
2019-09-06 12:43 ` Eric Sandeen
2019-09-06 13:00 ` David Howells
2019-09-06 13:01 ` David Howells
1 sibling, 2 replies; 5+ messages in thread
From: David Howells @ 2019-09-06 9:17 UTC (permalink / raw)
To: Eric Sandeen; +Cc: dhowells, fsdevel
Eric Sandeen <sandeen@redhat.com> wrote:
> There doesn't seem to be a strong reason to have another copy of the
> filesystem name string in the fs_parameter_description structure;
> it's easy enough to get the name from the fs_type, and using it
> instead ensures consistency across messages (for example,
> vfs_parse_fs_param() already uses fc->fs_type->name for the error
> messages, because it doesn't have the fs_parameter_description).
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
It was put there for fs_validate_description() to use. That checks both
filesystem and LSM parameter descriptions.
We could pass a name in to that function instead.
David
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fs: fs_parser: remove fs_parameter_description name field
2019-09-06 9:17 ` David Howells
@ 2019-09-06 12:43 ` Eric Sandeen
2019-09-06 13:00 ` David Howells
1 sibling, 0 replies; 5+ messages in thread
From: Eric Sandeen @ 2019-09-06 12:43 UTC (permalink / raw)
To: David Howells; +Cc: fsdevel
On 9/6/19 4:17 AM, David Howells wrote:
> Eric Sandeen <sandeen@redhat.com> wrote:
>
>> There doesn't seem to be a strong reason to have another copy of the
>> filesystem name string in the fs_parameter_description structure;
>> it's easy enough to get the name from the fs_type, and using it
>> instead ensures consistency across messages (for example,
>> vfs_parse_fs_param() already uses fc->fs_type->name for the error
>> messages, because it doesn't have the fs_parameter_description).
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>
> It was put there for fs_validate_description() to use. That checks both
> filesystem and LSM parameter descriptions.
>
> We could pass a name in to that function instead.
My patch does exactly that, right?
diff --git a/fs/filesystems.c b/fs/filesystems.c
index 9135646e41ac..77bf5f95362d 100644
--- a/fs/filesystems.c
+++ b/fs/filesystems.c
@@ -74,7 +74,8 @@ int register_filesystem(struct file_system_type * fs)
int res = 0;
struct file_system_type ** p;
- if (fs->parameters && !fs_validate_description(fs->parameters))
+ if (fs->parameters &&
+ !fs_validate_description(fs->name, fs->parameters))
return -EINVAL;
....
@@ -7021,7 +7020,7 @@ static __init int selinux_init(void)
else
pr_debug("SELinux: Starting in permissive mode\n");
- fs_validate_description(&selinux_fs_parameters);
+ fs_validate_description("selinux", &selinux_fs_parameters);
return 0;
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] fs: fs_parser: remove fs_parameter_description name field
2019-09-06 9:17 ` David Howells
2019-09-06 12:43 ` Eric Sandeen
@ 2019-09-06 13:00 ` David Howells
1 sibling, 0 replies; 5+ messages in thread
From: David Howells @ 2019-09-06 13:00 UTC (permalink / raw)
To: Eric Sandeen; +Cc: dhowells, fsdevel
Eric Sandeen <sandeen@redhat.com> wrote:
> My patch does exactly that, right?
Sorry, I missed that. Distracted by Linus.
David
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fs: fs_parser: remove fs_parameter_description name field
2019-08-27 0:46 [PATCH] fs: fs_parser: remove fs_parameter_description name field Eric Sandeen
2019-09-06 9:17 ` David Howells
@ 2019-09-06 13:01 ` David Howells
1 sibling, 0 replies; 5+ messages in thread
From: David Howells @ 2019-09-06 13:01 UTC (permalink / raw)
To: Al Viro; +Cc: dhowells, Eric Sandeen, fsdevel
Eric Sandeen <sandeen@redhat.com> wrote:
> There doesn't seem to be a strong reason to have another copy of the
> filesystem name string in the fs_parameter_description structure;
> it's easy enough to get the name from the fs_type, and using it
> instead ensures consistency across messages (for example,
> vfs_parse_fs_param() already uses fc->fs_type->name for the error
> messages, because it doesn't have the fs_parameter_description).
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Acked-by: David Howells <dhowells@redhat.com>
Al, can you add this to your branch?
David
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-09-06 13:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-27 0:46 [PATCH] fs: fs_parser: remove fs_parameter_description name field Eric Sandeen
2019-09-06 9:17 ` David Howells
2019-09-06 12:43 ` Eric Sandeen
2019-09-06 13:00 ` David Howells
2019-09-06 13:01 ` David Howells
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).