* [PATCH] sysctl: move local variable in proc_do_large_bitmap() to proper scope
@ 2020-11-09 7:11 Lukas Bulwahn
2020-11-10 1:08 ` Nathan Chancellor
2020-11-10 1:26 ` Tom Rix
0 siblings, 2 replies; 4+ messages in thread
From: Lukas Bulwahn @ 2020-11-09 7:11 UTC (permalink / raw)
To: Luis Chamberlain, Kees Cook, Iurii Zaikin, linux-fsdevel
Cc: Tom Rix, Nathan Chancellor, Nick Desaulniers, clang-built-linux,
kernel-janitors, linux-safety, linux-kernel, Lukas Bulwahn
make clang-analyzer caught my attention with:
kernel/sysctl.c:1511:4: warning: Value stored to 'first' is never read \
[clang-analyzer-deadcode.DeadStores]
first = 0;
^
Commit 9f977fb7ae9d ("sysctl: add proc_do_large_bitmap") introduced
proc_do_large_bitmap(), where the variable first is only effectively used
when write is false; when write is true, the variable first is only used in
a dead assignment.
So, simply remove this dead assignment and put the variable in local scope.
As compilers will detect this unneeded assignment and optimize this anyway,
the resulting object code is identical before and after this change.
No functional change. No change to object code.
Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
---
applies cleanly on v5.10-rc3 and next-20201106
Luis, Kees, Iurii, please pick this minor non-urgent clean-up patch.
kernel/sysctl.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index ce75c67572b9..cc274a431d91 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1423,7 +1423,6 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
void *buffer, size_t *lenp, loff_t *ppos)
{
int err = 0;
- bool first = 1;
size_t left = *lenp;
unsigned long bitmap_len = table->maxlen;
unsigned long *bitmap = *(unsigned long **) table->data;
@@ -1508,12 +1507,12 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
}
bitmap_set(tmp_bitmap, val_a, val_b - val_a + 1);
- first = 0;
proc_skip_char(&p, &left, '\n');
}
left += skipped;
} else {
unsigned long bit_a, bit_b = 0;
+ bool first = 1;
while (left) {
bit_a = find_next_bit(bitmap, bitmap_len, bit_b);
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] sysctl: move local variable in proc_do_large_bitmap() to proper scope
2020-11-09 7:11 [PATCH] sysctl: move local variable in proc_do_large_bitmap() to proper scope Lukas Bulwahn
@ 2020-11-10 1:08 ` Nathan Chancellor
2020-11-10 1:26 ` Tom Rix
1 sibling, 0 replies; 4+ messages in thread
From: Nathan Chancellor @ 2020-11-10 1:08 UTC (permalink / raw)
To: Lukas Bulwahn
Cc: Luis Chamberlain, Kees Cook, Iurii Zaikin, linux-fsdevel,
Tom Rix, Nick Desaulniers, clang-built-linux, kernel-janitors,
linux-safety, linux-kernel
On Mon, Nov 09, 2020 at 08:11:07AM +0100, Lukas Bulwahn wrote:
> make clang-analyzer caught my attention with:
>
> kernel/sysctl.c:1511:4: warning: Value stored to 'first' is never read \
> [clang-analyzer-deadcode.DeadStores]
> first = 0;
> ^
>
> Commit 9f977fb7ae9d ("sysctl: add proc_do_large_bitmap") introduced
> proc_do_large_bitmap(), where the variable first is only effectively used
> when write is false; when write is true, the variable first is only used in
> a dead assignment.
>
> So, simply remove this dead assignment and put the variable in local scope.
>
> As compilers will detect this unneeded assignment and optimize this anyway,
> the resulting object code is identical before and after this change.
>
> No functional change. No change to object code.
>
> Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> applies cleanly on v5.10-rc3 and next-20201106
>
> Luis, Kees, Iurii, please pick this minor non-urgent clean-up patch.
>
> kernel/sysctl.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index ce75c67572b9..cc274a431d91 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1423,7 +1423,6 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
> void *buffer, size_t *lenp, loff_t *ppos)
> {
> int err = 0;
> - bool first = 1;
> size_t left = *lenp;
> unsigned long bitmap_len = table->maxlen;
> unsigned long *bitmap = *(unsigned long **) table->data;
> @@ -1508,12 +1507,12 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
> }
>
> bitmap_set(tmp_bitmap, val_a, val_b - val_a + 1);
> - first = 0;
> proc_skip_char(&p, &left, '\n');
> }
> left += skipped;
> } else {
> unsigned long bit_a, bit_b = 0;
> + bool first = 1;
>
> while (left) {
> bit_a = find_next_bit(bitmap, bitmap_len, bit_b);
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] sysctl: move local variable in proc_do_large_bitmap() to proper scope
2020-11-09 7:11 [PATCH] sysctl: move local variable in proc_do_large_bitmap() to proper scope Lukas Bulwahn
2020-11-10 1:08 ` Nathan Chancellor
@ 2020-11-10 1:26 ` Tom Rix
2020-11-13 14:17 ` Luis Chamberlain
1 sibling, 1 reply; 4+ messages in thread
From: Tom Rix @ 2020-11-10 1:26 UTC (permalink / raw)
To: Lukas Bulwahn, Luis Chamberlain, Kees Cook, Iurii Zaikin, linux-fsdevel
Cc: Nathan Chancellor, Nick Desaulniers, clang-built-linux,
kernel-janitors, linux-safety, linux-kernel
On 11/8/20 11:11 PM, Lukas Bulwahn wrote:
> make clang-analyzer caught my attention with:
>
> kernel/sysctl.c:1511:4: warning: Value stored to 'first' is never read \
> [clang-analyzer-deadcode.DeadStores]
> first = 0;
> ^
>
> Commit 9f977fb7ae9d ("sysctl: add proc_do_large_bitmap") introduced
> proc_do_large_bitmap(), where the variable first is only effectively used
> when write is false; when write is true, the variable first is only used in
> a dead assignment.
>
> So, simply remove this dead assignment and put the variable in local scope.
>
> As compilers will detect this unneeded assignment and optimize this anyway,
> the resulting object code is identical before and after this change.
>
> No functional change. No change to object code.
>
> Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> ---
> applies cleanly on v5.10-rc3 and next-20201106
>
> Luis, Kees, Iurii, please pick this minor non-urgent clean-up patch.
>
> kernel/sysctl.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index ce75c67572b9..cc274a431d91 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1423,7 +1423,6 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
> void *buffer, size_t *lenp, loff_t *ppos)
> {
> int err = 0;
> - bool first = 1;
> size_t left = *lenp;
> unsigned long bitmap_len = table->maxlen;
> unsigned long *bitmap = *(unsigned long **) table->data;
> @@ -1508,12 +1507,12 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
> }
>
> bitmap_set(tmp_bitmap, val_a, val_b - val_a + 1);
> - first = 0;
> proc_skip_char(&p, &left, '\n');
> }
> left += skipped;
> } else {
> unsigned long bit_a, bit_b = 0;
> + bool first = 1;
This looks fine, but while you are here how about setting, to match the type
first = true
And then only clearing first once
if (!first)
proc_put_char(&buffer, &left, ',');
else
first = false
Instead of at every loop iteraction
Tom
>
> while (left) {
> bit_a = find_next_bit(bitmap, bitmap_len, bit_b);
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] sysctl: move local variable in proc_do_large_bitmap() to proper scope
2020-11-10 1:26 ` Tom Rix
@ 2020-11-13 14:17 ` Luis Chamberlain
0 siblings, 0 replies; 4+ messages in thread
From: Luis Chamberlain @ 2020-11-13 14:17 UTC (permalink / raw)
To: Tom Rix, Andrew Morton
Cc: Lukas Bulwahn, Kees Cook, Iurii Zaikin, linux-fsdevel,
Nathan Chancellor, Nick Desaulniers, clang-built-linux,
kernel-janitors, linux-safety, linux-kernel
On Mon, Nov 09, 2020 at 05:26:37PM -0800, Tom Rix wrote:
>
> On 11/8/20 11:11 PM, Lukas Bulwahn wrote:
> > make clang-analyzer caught my attention with:
> >
> > kernel/sysctl.c:1511:4: warning: Value stored to 'first' is never read \
> > [clang-analyzer-deadcode.DeadStores]
> > first = 0;
> > ^
> >
> > Commit 9f977fb7ae9d ("sysctl: add proc_do_large_bitmap") introduced
> > proc_do_large_bitmap(), where the variable first is only effectively used
> > when write is false; when write is true, the variable first is only used in
> > a dead assignment.
> >
> > So, simply remove this dead assignment and put the variable in local scope.
> >
> > As compilers will detect this unneeded assignment and optimize this anyway,
> > the resulting object code is identical before and after this change.
> >
> > No functional change. No change to object code.
> >
> > Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> > ---
> > applies cleanly on v5.10-rc3 and next-20201106
> >
> > Luis, Kees, Iurii, please pick this minor non-urgent clean-up patch.
> >
> > kernel/sysctl.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > index ce75c67572b9..cc274a431d91 100644
> > --- a/kernel/sysctl.c
> > +++ b/kernel/sysctl.c
> > @@ -1423,7 +1423,6 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
> > void *buffer, size_t *lenp, loff_t *ppos)
> > {
> > int err = 0;
> > - bool first = 1;
> > size_t left = *lenp;
> > unsigned long bitmap_len = table->maxlen;
> > unsigned long *bitmap = *(unsigned long **) table->data;
> > @@ -1508,12 +1507,12 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
> > }
> >
> > bitmap_set(tmp_bitmap, val_a, val_b - val_a + 1);
> > - first = 0;
> > proc_skip_char(&p, &left, '\n');
> > }
> > left += skipped;
> > } else {
> > unsigned long bit_a, bit_b = 0;
> > + bool first = 1;
>
> This looks fine, but while you are here how about setting, to match the type
>
> first = true
>
> And then only clearing first once
>
> if (!first)
> proc_put_char(&buffer, &left, ',');
>
> else
>
> first = false
>
> Instead of at every loop iteraction
Thanks for your patch Lukas!
Agreed, please resend with that change as requested by Tom. And also
please add Andrew Morton <akpm@linux-foundation.org> to your To address.
Luis
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-13 14:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-09 7:11 [PATCH] sysctl: move local variable in proc_do_large_bitmap() to proper scope Lukas Bulwahn
2020-11-10 1:08 ` Nathan Chancellor
2020-11-10 1:26 ` Tom Rix
2020-11-13 14:17 ` Luis Chamberlain
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).