* [PATCH] params: make dashes and underscores in parameter names truly equal
@ 2011-10-09 22:03 Michal Schmidt
2011-10-12 23:28 ` Rusty Russell
0 siblings, 1 reply; 6+ messages in thread
From: Michal Schmidt @ 2011-10-09 22:03 UTC (permalink / raw)
To: linux-kernel; +Cc: Rusty Russell, Randy Dunlap, Michal Schmidt
The user may use "foo-bar" for a kernel parameter defined as "foo_bar".
Make sure it works the other way around too.
Apply the equality of dashes and underscores on early_params and __setup
params as well.
The example given in Documentation/kernel-parameters.txt indicates that
this is the intended behaviour.
With the patch the kernel accepts "log-buf-len=1M" as expected.
https://bugzilla.redhat.com/show_bug.cgi?id=744545
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
include/linux/moduleparam.h | 20 ++++++++++++++++++++
init/main.c | 4 ++--
kernel/params.c | 34 +++++++++++++++++++++++++++-------
3 files changed, 49 insertions(+), 9 deletions(-)
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 56bda92..609f89c 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -267,6 +267,26 @@ static inline void __kernel_param_unlock(void)
.str = &__param_string_##name, 0, perm); \
__MODULE_PARM_TYPE(name, "string")
+/**
+ * parameq - checks if two parameter names match
+ * @name1: parameter name 1
+ * @name2: parameter name 2
+ *
+ * Returns true if the two parameter names are equal.
+ * Dashes (-) are considered equal to underscores (_).
+ */
+extern bool parameq(const char *name1, const char *name2);
+
+/**
+ * parameqn - checks if two parameter names match
+ * @name1: parameter name 1
+ * @name2: parameter name 2
+ * @n: the maximum length to compare
+ *
+ * Similar to parameq(), except it compares at most @n characters.
+ */
+extern bool parameqn(const char *name1, const char *name2, size_t n);
+
/* Called on module insert or kernel boot */
extern int parse_args(const char *name,
char *args,
diff --git a/init/main.c b/init/main.c
index 03b408d..63f5f6f 100644
--- a/init/main.c
+++ b/init/main.c
@@ -163,7 +163,7 @@ static int __init obsolete_checksetup(char *line)
p = __setup_start;
do {
int n = strlen(p->str);
- if (!strncmp(line, p->str, n)) {
+ if (parameqn(line, p->str, n)) {
if (p->early) {
/* Already done in parse_early_param?
* (Needs exact match on param part).
@@ -392,7 +392,7 @@ static int __init do_early_param(char *param, char *val)
const struct obs_kernel_param *p;
for (p = __setup_start; p < __setup_end; p++) {
- if ((p->early && strcmp(param, p->str) == 0) ||
+ if ((p->early && parameq(param, p->str)) ||
(strcmp(param, "console") == 0 &&
strcmp(p->str, "earlycon") == 0)
) {
diff --git a/kernel/params.c b/kernel/params.c
index 1d1c01b..aa8933c 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -67,20 +67,40 @@ static void maybe_kfree_parameter(void *param)
}
}
-static inline char dash2underscore(char c)
+static char dash2underscore(char c)
{
if (c == '-')
return '_';
return c;
}
-static inline int parameq(const char *input, const char *paramname)
+bool parameq(const char *name1, const char *name2)
{
- unsigned int i;
- for (i = 0; dash2underscore(input[i]) == paramname[i]; i++)
- if (input[i] == '\0')
- return 1;
- return 0;
+ char c1, c2;
+ while (1) {
+ c1 = dash2underscore(*name1++);
+ c2 = dash2underscore(*name2++);
+ if (c1 != c2)
+ return false;
+ if (c1 == '\0')
+ break;
+ }
+ return true;
+}
+
+bool parameqn(const char *name1, const char *name2, size_t n)
+{
+ char c1, c2;
+ while (n) {
+ c1 = dash2underscore(*name1++);
+ c2 = dash2underscore(*name2++);
+ if (c1 != c2)
+ return false;
+ if (c1 == '\0')
+ break;
+ n--;
+ }
+ return true;
}
static int parse_one(char *param,
--
1.7.4.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] params: make dashes and underscores in parameter names truly equal
2011-10-09 22:03 [PATCH] params: make dashes and underscores in parameter names truly equal Michal Schmidt
@ 2011-10-12 23:28 ` Rusty Russell
2011-10-13 14:41 ` Michal Schmidt
0 siblings, 1 reply; 6+ messages in thread
From: Rusty Russell @ 2011-10-12 23:28 UTC (permalink / raw)
To: Michal Schmidt, linux-kernel; +Cc: Randy Dunlap, Michal Schmidt
On Mon, 10 Oct 2011 00:03:37 +0200, Michal Schmidt <mschmidt@redhat.com> wrote:
> The user may use "foo-bar" for a kernel parameter defined as "foo_bar".
> Make sure it works the other way around too.
Hi Michal,
Idea is solid, implementation has a few quirks. As the name
obsolete_checksetup() implies, this was only supposed to be a crutch for
old __setup() calls. Modern code should be using module_param() or
core_param().
Did you have a specific example? Perhaps we should work on converting
them all, which would have beneficial side-effects as we poke into old
code...
Thanks!
Rusty.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] params: make dashes and underscores in parameter names truly equal
2011-10-12 23:28 ` Rusty Russell
@ 2011-10-13 14:41 ` Michal Schmidt
2011-10-18 3:58 ` Rusty Russell
0 siblings, 1 reply; 6+ messages in thread
From: Michal Schmidt @ 2011-10-13 14:41 UTC (permalink / raw)
To: Rusty Russell; +Cc: linux-kernel, Randy Dunlap
On 10/13/2011 01:28 AM, Rusty Russell wrote:
> Idea is solid, implementation has a few quirks. As the name
> obsolete_checksetup() implies, this was only supposed to be a crutch for
> old __setup() calls. Modern code should be using module_param() or
> core_param().
... instead of __setup()?
No objection to the change in do_early_param() ?
> Did you have a specific example?
The specific example I had in mind was the one from
Documentation/kernel-parameters.txt:
log_buf_len=1M print-fatal-signals=1
can also be entered as
log-buf-len=1M print_fatal_signals=1
'log-buf-len=1M' does not work because it's an early_param().
'print_fatal_signals=1' does not work because it's defined as
__setup("print-fatal-signals=", ...);
> Perhaps we should work on converting
> them all, which would have beneficial side-effects as we poke into old
> code...
Let's see how many __setup() parameters there are:
$ git grep -E '^__setup\("' |wc -l
457
Only the ones with a dash or underscore in the name:
$ git grep -E '__setup\(".*[_-].*"' | wc -l
180
It will take some time.
Thanks,
Michal
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] params: make dashes and underscores in parameter names truly equal
2011-10-13 14:41 ` Michal Schmidt
@ 2011-10-18 3:58 ` Rusty Russell
2011-10-18 19:01 ` Randy Dunlap
0 siblings, 1 reply; 6+ messages in thread
From: Rusty Russell @ 2011-10-18 3:58 UTC (permalink / raw)
To: Michal Schmidt; +Cc: linux-kernel, Randy Dunlap
On Thu, 13 Oct 2011 16:41:11 +0200, Michal Schmidt <mschmidt@redhat.com> wrote:
> On 10/13/2011 01:28 AM, Rusty Russell wrote:
> > Idea is solid, implementation has a few quirks. As the name
> > obsolete_checksetup() implies, this was only supposed to be a crutch for
> > old __setup() calls. Modern code should be using module_param() or
> > core_param().
>
> ... instead of __setup()?
Yep.
> No objection to the change in do_early_param() ?
Well, for the moment I've applied that, with parameq and parameqn
reimplemented more cleanly. See below.
> > Did you have a specific example?
>
> The specific example I had in mind was the one from
> Documentation/kernel-parameters.txt:
>
> log_buf_len=1M print-fatal-signals=1
> can also be entered as
> log-buf-len=1M print_fatal_signals=1
Oops.
> Let's see how many __setup() parameters there are:
> $ git grep -E '^__setup\("' |wc -l
> 457
>
> Only the ones with a dash or underscore in the name:
> $ git grep -E '__setup\(".*[_-].*"' | wc -l
> 180
>
> It will take some time.
The easy things have already been done ;)
Just do the non-arch ones, first, and see where that gets us.
Thanks!
Rusty.
From: Michal Schmidt <mschmidt@redhat.com>
Subject: params: make dashes and underscores in parameter names truly equal
Date: Mon, 10 Oct 2011 00:03:37 +0200
The user may use "foo-bar" for a kernel parameter defined as "foo_bar".
Make sure it works the other way around too.
Apply the equality of dashes and underscores on early_params and __setup
params as well.
The example given in Documentation/kernel-parameters.txt indicates that
this is the intended behaviour.
With the patch the kernel accepts "log-buf-len=1M" as expected.
https://bugzilla.redhat.com/show_bug.cgi?id=744545
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> (neatened implementations)
---
include/linux/moduleparam.h | 20 ++++++++++++++++++++
init/main.c | 4 ++--
kernel/params.c | 21 ++++++++++++++-------
3 files changed, 36 insertions(+), 9 deletions(-)
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -262,6 +262,26 @@ static inline void __kernel_param_unlock
.str = &__param_string_##name, 0, perm); \
__MODULE_PARM_TYPE(name, "string")
+/**
+ * parameq - checks if two parameter names match
+ * @name1: parameter name 1
+ * @name2: parameter name 2
+ *
+ * Returns true if the two parameter names are equal.
+ * Dashes (-) are considered equal to underscores (_).
+ */
+extern bool parameq(const char *name1, const char *name2);
+
+/**
+ * parameqn - checks if two parameter names match
+ * @name1: parameter name 1
+ * @name2: parameter name 2
+ * @n: the length to compare
+ *
+ * Similar to parameq(), except it compares @n characters.
+ */
+extern bool parameqn(const char *name1, const char *name2, size_t n);
+
/* Called on module insert or kernel boot */
extern int parse_args(const char *name,
char *args,
diff --git a/init/main.c b/init/main.c
--- a/init/main.c
+++ b/init/main.c
@@ -163,7 +163,7 @@ static int __init obsolete_checksetup(ch
p = __setup_start;
do {
int n = strlen(p->str);
- if (!strncmp(line, p->str, n)) {
+ if (parameqn(line, p->str, n)) {
if (p->early) {
/* Already done in parse_early_param?
* (Needs exact match on param part).
@@ -392,7 +392,7 @@ static int __init do_early_param(char *p
const struct obs_kernel_param *p;
for (p = __setup_start; p < __setup_end; p++) {
- if ((p->early && strcmp(param, p->str) == 0) ||
+ if ((p->early && parameq(param, p->str)) ||
(strcmp(param, "console") == 0 &&
strcmp(p->str, "earlycon") == 0)
) {
diff --git a/kernel/params.c b/kernel/params.c
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -67,20 +67,27 @@ static void maybe_kfree_parameter(void *
}
}
-static inline char dash2underscore(char c)
+static char dash2underscore(char c)
{
if (c == '-')
return '_';
return c;
}
-static inline int parameq(const char *input, const char *paramname)
+bool parameqn(const char *a, const char *b, size_t n)
{
- unsigned int i;
- for (i = 0; dash2underscore(input[i]) == paramname[i]; i++)
- if (input[i] == '\0')
- return 1;
- return 0;
+ size_t i;
+
+ for (i = 0; i < n; i++) {
+ if (dash2underscore(a[i]) != dash2underscore(b[i]))
+ return false;
+ }
+ return true;
+}
+
+bool parameq(const char *a, const char *b)
+{
+ return parameqn(a, b, strlen(a)+1);
}
static int parse_one(char *param,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] params: make dashes and underscores in parameter names truly equal
2011-10-18 3:58 ` Rusty Russell
@ 2011-10-18 19:01 ` Randy Dunlap
2011-10-19 3:00 ` Rusty Russell
0 siblings, 1 reply; 6+ messages in thread
From: Randy Dunlap @ 2011-10-18 19:01 UTC (permalink / raw)
To: Rusty Russell; +Cc: Michal Schmidt, linux-kernel
On 10/17/2011 08:58 PM, Rusty Russell wrote:
> On Thu, 13 Oct 2011 16:41:11 +0200, Michal Schmidt <mschmidt@redhat.com> wrote:
>> On 10/13/2011 01:28 AM, Rusty Russell wrote:
>>> Idea is solid, implementation has a few quirks. As the name
>>> obsolete_checksetup() implies, this was only supposed to be a crutch for
>>> old __setup() calls. Modern code should be using module_param() or
>>> core_param().
>>
>> ... instead of __setup()?
>
> Yep.
>
>> No objection to the change in do_early_param() ?
>
> Well, for the moment I've applied that, with parameq and parameqn
> reimplemented more cleanly. See below.
>
>>> Did you have a specific example?
>>
>> The specific example I had in mind was the one from
>> Documentation/kernel-parameters.txt:
>>
>> log_buf_len=1M print-fatal-signals=1
>> can also be entered as
>> log-buf-len=1M print_fatal_signals=1
>
> Oops.
>
>> Let's see how many __setup() parameters there are:
>> $ git grep -E '^__setup\("' |wc -l
>> 457
>>
>> Only the ones with a dash or underscore in the name:
>> $ git grep -E '__setup\(".*[_-].*"' | wc -l
>> 180
>>
>> It will take some time.
>
> The easy things have already been done ;)
>
> Just do the non-arch ones, first, and see where that gets us.
>
> Thanks!
> Rusty.
>
> From: Michal Schmidt <mschmidt@redhat.com>
> Subject: params: make dashes and underscores in parameter names truly equal
> Date: Mon, 10 Oct 2011 00:03:37 +0200
>
> The user may use "foo-bar" for a kernel parameter defined as "foo_bar".
> Make sure it works the other way around too.
>
> Apply the equality of dashes and underscores on early_params and __setup
> params as well.
>
> The example given in Documentation/kernel-parameters.txt indicates that
> this is the intended behaviour.
Well, that could have just been my misunderstanding, but I think that this
patch is the right thing to do. Thanks.
> With the patch the kernel accepts "log-buf-len=1M" as expected.
> https://bugzilla.redhat.com/show_bug.cgi?id=744545
>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> (neatened implementations)
> ---
> include/linux/moduleparam.h | 20 ++++++++++++++++++++
> init/main.c | 4 ++--
> kernel/params.c | 21 ++++++++++++++-------
> 3 files changed, 36 insertions(+), 9 deletions(-)
--
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] params: make dashes and underscores in parameter names truly equal
2011-10-18 19:01 ` Randy Dunlap
@ 2011-10-19 3:00 ` Rusty Russell
0 siblings, 0 replies; 6+ messages in thread
From: Rusty Russell @ 2011-10-19 3:00 UTC (permalink / raw)
To: Randy Dunlap; +Cc: Michal Schmidt, linux-kernel
On Tue, 18 Oct 2011 12:01:00 -0700, Randy Dunlap <rdunlap@xenotime.net> wrote:
> Well, that could have just been my misunderstanding, but I think that this
> patch is the right thing to do. Thanks.
It does two things:
1) It fixes early_params to be case-insensitive. Good.
2) It fixes the (obsolete) __setup() to be case-insensitive.
We should finally get rid of __setup, then we wouldn't need the second
part.
Cheers,
Rusty.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-10-19 3:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-09 22:03 [PATCH] params: make dashes and underscores in parameter names truly equal Michal Schmidt
2011-10-12 23:28 ` Rusty Russell
2011-10-13 14:41 ` Michal Schmidt
2011-10-18 3:58 ` Rusty Russell
2011-10-18 19:01 ` Randy Dunlap
2011-10-19 3:00 ` Rusty Russell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox