* proc_dointvec() and write
@ 2010-05-24 14:32 J. R. Okajima
2010-05-25 4:49 ` Cong Wang
0 siblings, 1 reply; 8+ messages in thread
From: J. R. Okajima @ 2010-05-24 14:32 UTC (permalink / raw)
To: amwang, opurdila, davem; +Cc: linux-kernel
The commit 00b7c3395aec3df43de5bd02a3c5a099ca51169f
"sysctl: refactor integer handling proc code"
modified the behaviour of writing to /proc.
Before the commit, write("1\n") to /proc/sys/kernel/printk succeeded. But
now it returns EINVAL. Is this intended change? If not, how about this
patch?
J. R. Okajima
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 4c93486..5058e12 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2262,6 +2272,8 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
if (write) {
left -= proc_skip_spaces(&kbuf);
+ if (!left)
+ break;
err = proc_get_long(&kbuf, &left, &lval, &neg,
proc_wspace_sep,
sizeof(proc_wspace_sep), NULL);
@@ -2288,7 +2306,7 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
if (!write && !first && left && !err)
err = proc_put_char(&buffer, &left, '\n');
- if (write && !err)
+ if (write && !err && left)
left -= proc_skip_spaces(&kbuf);
free:
if (write) {
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: proc_dointvec() and write
2010-05-24 14:32 proc_dointvec() and write J. R. Okajima
@ 2010-05-25 4:49 ` Cong Wang
2010-05-25 4:57 ` J. R. Okajima
0 siblings, 1 reply; 8+ messages in thread
From: Cong Wang @ 2010-05-25 4:49 UTC (permalink / raw)
To: J. R. Okajima; +Cc: opurdila, davem, linux-kernel
On 05/24/10 22:32, J. R. Okajima wrote:
> The commit 00b7c3395aec3df43de5bd02a3c5a099ca51169f
> "sysctl: refactor integer handling proc code"
> modified the behaviour of writing to /proc.
>
> Before the commit, write("1\n") to /proc/sys/kernel/printk succeeded. But
> now it returns EINVAL. Is this intended change? If not, how about this
> patch?
>
Hmm, odd, I tested other proc files, I see no problem.
I am wondering why /proc/sys/kernel/printk is special here.
I will be back later.
>
> J. R. Okajima
>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 4c93486..5058e12 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2262,6 +2272,8 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
> if (write) {
> left -= proc_skip_spaces(&kbuf);
>
> + if (!left)
> + break;
> err = proc_get_long(&kbuf,&left,&lval,&neg,
> proc_wspace_sep,
> sizeof(proc_wspace_sep), NULL);
> @@ -2288,7 +2306,7 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
>
> if (!write&& !first&& left&& !err)
> err = proc_put_char(&buffer,&left, '\n');
> - if (write&& !err)
> + if (write&& !err&& left)
> left -= proc_skip_spaces(&kbuf);
> free:
> if (write) {
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: proc_dointvec() and write
2010-05-25 4:49 ` Cong Wang
@ 2010-05-25 4:57 ` J. R. Okajima
2010-05-25 5:35 ` Cong Wang
0 siblings, 1 reply; 8+ messages in thread
From: J. R. Okajima @ 2010-05-25 4:57 UTC (permalink / raw)
To: Cong Wang; +Cc: opurdila, davem, linux-kernel
Cong Wang:
> Hmm, odd, I tested other proc files, I see no problem.
> I am wondering why /proc/sys/kernel/printk is special here.
The condition is,
- the entry has multiple values, .maxlen = N * sizeof(foo)
- the entry is writable, .mode = 0644 (or something)
In sysctl.c, only "printk" and "acct" match.
J. R. Okajima
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: proc_dointvec() and write
2010-05-25 4:57 ` J. R. Okajima
@ 2010-05-25 5:35 ` Cong Wang
2010-05-25 6:49 ` [PATCH] proc_dointvec, write a single value J. R. Okajima
0 siblings, 1 reply; 8+ messages in thread
From: Cong Wang @ 2010-05-25 5:35 UTC (permalink / raw)
To: J. R. Okajima; +Cc: opurdila, davem, linux-kernel
On 05/25/10 12:57, J. R. Okajima wrote:
> Cong Wang:
>> Hmm, odd, I tested other proc files, I see no problem.
>> I am wondering why /proc/sys/kernel/printk is special here.
>
> The condition is,
> - the entry has multiple values, .maxlen = N * sizeof(foo)
> - the entry is writable, .mode = 0644 (or something)
>
> In sysctl.c, only "printk" and "acct" match.
Hmm, right, I missed this case during testing.
Your patch looks correct for me. Would you mind to send your
patch again with your Signed-off-by?
Thank you!
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] proc_dointvec, write a single value
2010-05-25 5:35 ` Cong Wang
@ 2010-05-25 6:49 ` J. R. Okajima
2010-05-25 12:23 ` Cong Wang
0 siblings, 1 reply; 8+ messages in thread
From: J. R. Okajima @ 2010-05-25 6:49 UTC (permalink / raw)
To: amwang; +Cc: opurdila, davem, linux-kernel, J. R. Okajima
The commit 00b7c3395aec3df43de5bd02a3c5a099ca51169f
"sysctl: refactor integer handling proc code"
modified the behaviour of writing to /proc.
Before the commit, write("1\n") to /proc/sys/kernel/printk succeeded. But
now it returns EINVAL.
This commit supports writing a single value to a multi-valued entry.
Signed-off-by: J. R. Okajima <hooanon05@yahoo.co.jp>
---
kernel/sysctl.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 4c93486..78ac3fb 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2262,6 +2262,8 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
if (write) {
left -= proc_skip_spaces(&kbuf);
+ if (!left)
+ break;
err = proc_get_long(&kbuf, &left, &lval, &neg,
proc_wspace_sep,
sizeof(proc_wspace_sep), NULL);
@@ -2288,7 +2290,7 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
if (!write && !first && left && !err)
err = proc_put_char(&buffer, &left, '\n');
- if (write && !err)
+ if (write && !err && left)
left -= proc_skip_spaces(&kbuf);
free:
if (write) {
--
1.6.6.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] proc_dointvec, write a single value
2010-05-25 6:49 ` [PATCH] proc_dointvec, write a single value J. R. Okajima
@ 2010-05-25 12:23 ` Cong Wang
2010-05-25 23:10 ` David Miller
0 siblings, 1 reply; 8+ messages in thread
From: Cong Wang @ 2010-05-25 12:23 UTC (permalink / raw)
To: J. R. Okajima; +Cc: opurdila, davem, linux-kernel
On 05/25/10 14:49, J. R. Okajima wrote:
> The commit 00b7c3395aec3df43de5bd02a3c5a099ca51169f
> "sysctl: refactor integer handling proc code"
> modified the behaviour of writing to /proc.
> Before the commit, write("1\n") to /proc/sys/kernel/printk succeeded. But
> now it returns EINVAL.
>
> This commit supports writing a single value to a multi-valued entry.
>
> Signed-off-by: J. R. Okajima<hooanon05@yahoo.co.jp>
Reviewed-and-tested-by: WANG Cong <amwang@redhat.com>
Thanks a lot!
> ---
> kernel/sysctl.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 4c93486..78ac3fb 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2262,6 +2262,8 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
> if (write) {
> left -= proc_skip_spaces(&kbuf);
>
> + if (!left)
> + break;
> err = proc_get_long(&kbuf,&left,&lval,&neg,
> proc_wspace_sep,
> sizeof(proc_wspace_sep), NULL);
> @@ -2288,7 +2290,7 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
>
> if (!write&& !first&& left&& !err)
> err = proc_put_char(&buffer,&left, '\n');
> - if (write&& !err)
> + if (write&& !err&& left)
> left -= proc_skip_spaces(&kbuf);
> free:
> if (write) {
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] proc_dointvec, write a single value
2010-05-25 12:23 ` Cong Wang
@ 2010-05-25 23:10 ` David Miller
2010-05-30 15:09 ` Eric W. Biederman
0 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2010-05-25 23:10 UTC (permalink / raw)
To: amwang; +Cc: hooanon05, opurdila, linux-kernel
From: Cong Wang <amwang@redhat.com>
Date: Tue, 25 May 2010 20:23:47 +0800
> On 05/25/10 14:49, J. R. Okajima wrote:
>> The commit 00b7c3395aec3df43de5bd02a3c5a099ca51169f
>> "sysctl: refactor integer handling proc code"
>> modified the behaviour of writing to /proc.
>> Before the commit, write("1\n") to /proc/sys/kernel/printk
>> succeeded. But
>> now it returns EINVAL.
>>
>> This commit supports writing a single value to a multi-valued entry.
>>
>> Signed-off-by: J. R. Okajima<hooanon05@yahoo.co.jp>
>
> Reviewed-and-tested-by: WANG Cong <amwang@redhat.com>
Since the regression causing change came in via the net tree
I'll integrate this fix too, applied, thanks!
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] proc_dointvec, write a single value
2010-05-25 23:10 ` David Miller
@ 2010-05-30 15:09 ` Eric W. Biederman
0 siblings, 0 replies; 8+ messages in thread
From: Eric W. Biederman @ 2010-05-30 15:09 UTC (permalink / raw)
To: David Miller; +Cc: amwang, hooanon05, opurdila, linux-kernel
David Miller <davem@davemloft.net> writes:
> From: Cong Wang <amwang@redhat.com>
> Date: Tue, 25 May 2010 20:23:47 +0800
>
>> On 05/25/10 14:49, J. R. Okajima wrote:
>>> The commit 00b7c3395aec3df43de5bd02a3c5a099ca51169f
>>> "sysctl: refactor integer handling proc code"
>>> modified the behaviour of writing to /proc.
>>> Before the commit, write("1\n") to /proc/sys/kernel/printk
>>> succeeded. But
>>> now it returns EINVAL.
>>>
>>> This commit supports writing a single value to a multi-valued entry.
>>>
>>> Signed-off-by: J. R. Okajima<hooanon05@yahoo.co.jp>
>>
>> Reviewed-and-tested-by: WANG Cong <amwang@redhat.com>
>
> Since the regression causing change came in via the net tree
> I'll integrate this fix too, applied, thanks!
Thanks, guys for fixing this.
I hate to see my review comments show up like this in practice.
I had really hoped those unnecessary changes to proc would have
been dropped from that patchset. Oh well.
Eric
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-05-30 15:09 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-24 14:32 proc_dointvec() and write J. R. Okajima
2010-05-25 4:49 ` Cong Wang
2010-05-25 4:57 ` J. R. Okajima
2010-05-25 5:35 ` Cong Wang
2010-05-25 6:49 ` [PATCH] proc_dointvec, write a single value J. R. Okajima
2010-05-25 12:23 ` Cong Wang
2010-05-25 23:10 ` David Miller
2010-05-30 15:09 ` Eric W. Biederman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.