| Message ID | 20241014183918.30123-1-quic_pintu@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/kmemleak: fix errors/warnings for coding style | expand |
On Tue, Oct 15, 2024 at 12:09:18AM +0530, Pintu Kumar wrote: > There are several errors/warnings reported by checkpatch. > Fix all of the positive once as below: > - Use #include <linux/processor.h> instead of <asm/processor.h> > - Missing a blank lines after declarations > - Prefer 'unsigned int' to bare use of 'unsigned' > - else should follow close brace '}' I wouldn't bother fixing up the style in existing code, it just makes backporting fixes harder. > @@ -2179,8 +2180,7 @@ static int __init kmemleak_boot_config(char *str) > else if (strcmp(str, "on") == 0) { > kmemleak_skip_disable = 1; > stack_depot_request_early_init(); > - } > - else > + } else > return -EINVAL; > return 0; According to the coding style doc, the 'else' branch also needs braces here.
Hi Catalin, On Thu, 17 Oct 2024 at 15:10, Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Tue, Oct 15, 2024 at 12:09:18AM +0530, Pintu Kumar wrote: > > There are several errors/warnings reported by checkpatch. > > Fix all of the positive once as below: > > - Use #include <linux/processor.h> instead of <asm/processor.h> > > - Missing a blank lines after declarations > > - Prefer 'unsigned int' to bare use of 'unsigned' > > - else should follow close brace '}' > > I wouldn't bother fixing up the style in existing code, it just makes > backporting fixes harder. > Thank you so much for your review. Yes, I agree but I see that this keeps growing. In 5.15 kernel there were 10 warnings: total: 0 errors, 10 warnings, 2010 lines checked But now in the 6.12 kernel it becomes 20. total: 1 errors, 20 warnings, 2253 lines checked This means new warnings were introduced while the old ones still remain. Thus I thought it's better to reduce it. > > @@ -2179,8 +2180,7 @@ static int __init kmemleak_boot_config(char *str) > > else if (strcmp(str, "on") == 0) { > > kmemleak_skip_disable = 1; > > stack_depot_request_early_init(); > > - } > > - else > > + } else > > return -EINVAL; > > return 0; > > According to the coding style doc, the 'else' branch also needs braces > here. Oh yes, thanks for pointing this out. Unfortunately checkpatch does not report this. But I tried adding braces and no issues either. I will push the new patchset with this change. Thanks, Pintu
diff --git a/mm/kmemleak.c b/mm/kmemleak.c index 0400f5e8ac60..11f92bb7b206 100644 --- a/mm/kmemleak.c +++ b/mm/kmemleak.c @@ -1,7 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * mm/kmemleak.c - * * Copyright (C) 2008 ARM Limited * Written by Catalin Marinas <catalin.marinas@arm.com> * @@ -95,7 +93,7 @@ #include <linux/crc32.h> #include <asm/sections.h> -#include <asm/processor.h> +#include <linux/processor.h> #include <linux/atomic.h> #include <linux/kasan.h> @@ -373,6 +371,7 @@ static void print_unreferenced(struct seq_file *seq, for (i = 0; i < nr_entries; i++) { void *ptr = (void *)entries[i]; + warn_or_seq_printf(seq, " [<%pK>] %pS\n", ptr, ptr); } } @@ -1711,6 +1710,7 @@ static void kmemleak_scan(void) rcu_read_lock(); for_each_process_thread(g, p) { void *stack = try_get_task_stack(p); + if (stack) { scan_block(stack, stack + THREAD_SIZE, NULL); put_task_stack(p); @@ -1817,6 +1817,7 @@ static int kmemleak_scan_thread(void *arg) */ if (first_run) { signed long timeout = msecs_to_jiffies(SECS_FIRST_SCAN * 1000); + first_run = 0; while (timeout && !kthread_should_stop()) timeout = schedule_timeout_interruptible(timeout); @@ -2062,7 +2063,7 @@ static ssize_t kmemleak_write(struct file *file, const char __user *user_buf, else if (strncmp(buf, "scan=off", 8) == 0) stop_scan_thread(); else if (strncmp(buf, "scan=", 5) == 0) { - unsigned secs; + unsigned int secs; unsigned long msecs; ret = kstrtouint(buf + 5, 0, &secs); @@ -2179,8 +2180,7 @@ static int __init kmemleak_boot_config(char *str) else if (strcmp(str, "on") == 0) { kmemleak_skip_disable = 1; stack_depot_request_early_init(); - } - else + } else return -EINVAL; return 0; }
There are several errors/warnings reported by checkpatch. Fix all of the positive once as below: - Use #include <linux/processor.h> instead of <asm/processor.h> - Missing a blank lines after declarations - Prefer 'unsigned int' to bare use of 'unsigned' - else should follow close brace '}' Signed-off-by: Pintu Kumar <quic_pintu@quicinc.com> --- mm/kmemleak.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)