sh: kprobes: remove unused variables in kprobe_exceptions_notify()
PATCH 1/1 sh: kprobes: remove unused variables in kprobe_exceptions_notify()
From: "Mike Rapoport (Microsoft)" <[email protected]>
kbuild reports the following warning:
arch/sh/kernel/kprobes.c: In function 'kprobe_exceptions_notify':
Quoted 1 line
Quoted 1 line
arch/sh/kernel/kprobes.c:412:24: warning: variable 'p' set but not used [-Wunused-but-set-variable]
412 | struct kprobe *p = NULL;
| ^The variable 'p' is indeed unused since the commit fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")
Remove that variable along with 'kprobe_opcode_t *addr' which also becomes unused after 'p' is removed.
---
I don't know why the warning poped up only now, the code there didn't change for some time :/
arch/sh/kernel/kprobes.c | 4 ---- 1 file changed, 4 deletions(-)
arch/sh/kernel/kprobes.c +0 -4
@@ -404,13 +404,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)int __kprobes kprobe_exceptions_notify (struct notifier_block *self,unsigned long val,void *data) { -struct kprobe *p =NULL ;struct die_args *args = (struct die_args *)data;int ret =NOTIFY_DONE ; -kprobe_opcode_t *addr =NULL ;struct kprobe_ctlblk *kcb =get_kprobe_ctlblk (); - addr = (kprobe_opcode_t *) (args->regs->pc);if (val ==DIE_TRAP && args->trapnr == (BREAKPOINT_INSTRUCTION &0xff )) {if (!kprobe_running ()) {
@@ -421,7 +418,6 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self, ret =NOTIFY_DONE ; } }else { - p =get_kprobe (addr);if ((kcb->kprobe_status ==KPROBE_HIT_SS ) || (kcb->kprobe_status ==KPROBE_REENTER )) {if (post_kprobe_handler (args->regs))
Signature · 1 line
2.47.2
Hi Mike,
Quoted 12 lines · Mike Rapoport
From: "Mike Rapoport (Microsoft)" <[email protected]>
kbuild reports the following warning:
arch/sh/kernel/kprobes.c: In function 'kprobe_exceptions_notify':Quoted 1 line
Quoted 1 line
arch/sh/kernel/kprobes.c:412:24: warning: variable 'p' set but not used [-Wunused-but-set-variable]412 | struct kprobe *p = NULL; | ^The variable 'p' is indeed unused since the commit
fa5a24b16f94("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")
Quoted 7 lines
Remove that variable along with '
kprobe_opcode_t*addr' which also becomes unused after 'p' is removed.Reported-by kernel test robot <[email protected]>Fixes fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")Signed-off-by Mike Rapoport (Microsoft) <[email protected]>
Thanks for your patch!
"p" and "addr" are definitely unused (besides side-effects?), so
Quoted 17 lines
arch/sh/kernel/kprobes.c +0
-3@@ -404,13 +404,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)int __kprobes kprobe_exceptions_notify (struct notifier_block *self,unsigned long val,void *data) { -struct kprobe *p =NULL ;struct die_args *args = (struct die_args *)data;int ret =NOTIFY_DONE ; -kprobe_opcode_t *addr =NULL ;struct kprobe_ctlblk *kcb =get_kprobe_ctlblk (); - addr = (kprobe_opcode_t *) (args->regs->pc);if (val ==DIE_TRAP && args->trapnr == (BREAKPOINT_INSTRUCTION &0xff )) {if (!kprobe_running ()) {
arch/sh/kernel/kprobes.c int __kprobes kprobe_exceptions_notify(struct notifier_block *self, · line 423
Quoted 7 lines
arch/sh/kernel/kprobes.c +0
-1@@ -421,7 +418,6 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self, ret =NOTIFY_DONE ; } }else { - p =get_kprobe (addr);if ((kcb->kprobe_status ==KPROBE_HIT_SS ) || (kcb->kprobe_status ==KPROBE_REENTER )) {if (post_kprobe_handler (args->regs))
I have no idea what this code is supposed to do, and if it actually works. Red flags are that the assigned "p" was never used at all since the inception of this function.
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Quoted 40 lines · Geert Uytterhoeven
Hi Mike,
Quoted 12 lines · Mike Rapoport
From: "Mike Rapoport (Microsoft)" <[email protected]>
kbuild reports the following warning:
arch/sh/kernel/kprobes.c: In function 'kprobe_exceptions_notify':Quoted 1 line
Quoted 1 line
arch/sh/kernel/kprobes.c:412:24: warning: variable 'p' set but not used [-Wunused-but-set-variable]412 | struct kprobe *p = NULL; | ^The variable 'p' is indeed unused since the commit
fa5a24b16f94("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")Quoted 7 lines
Remove that variable along with '
kprobe_opcode_t*addr' which also becomes unused after 'p' is removed.Reported-by kernel test robot <[email protected]>Fixes fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")Signed-off-by Mike Rapoport (Microsoft) <[email protected]>Thanks for your patch!
"p" and "addr" are definitely unused (besides side-effects?), so
Reviewed-by Geert Uytterhoeven <[email protected]>Quoted 12 lines
arch/sh/kernel/kprobes.c +0
-2@@ -404,13 +404,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)int __kprobes kprobe_exceptions_notify (struct notifier_block *self,unsigned long val,void *data) { -struct kprobe *p =NULL ;struct die_args *args = (struct die_args *)data;int ret =NOTIFY_DONE ; -kprobe_opcode_t *addr =NULL ;struct kprobe_ctlblk *kcb =get_kprobe_ctlblk ();
Quoted 16 lines
arch/sh/kernel/kprobes.c int __kprobes kprobe_exceptions_notify(struct notifier_block *self, · line 423
Quoted 12 lines
arch/sh/kernel/kprobes.c +0
-2- addr = (kprobe_opcode_t *) (args->regs->pc);if (val ==DIE_TRAP && args->trapnr == (BREAKPOINT_INSTRUCTION &0xff )) {if (!kprobe_running ()) {@@ -421,7 +418,6 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self, ret =NOTIFY_DONE ; } }else { - p =get_kprobe (addr);if ((kcb->kprobe_status ==KPROBE_HIT_SS ) || (kcb->kprobe_status ==KPROBE_REENTER )) {if (post_kprobe_handler (args->regs))I have no idea what this code is supposed to do, and if it actually works. Red flags are that the assigned "p" was never used at all since the inception of this function.
"p" was used before fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code"), but I can't say I understand that code either :)
CC'ing Masami, he probably knows what this code does.
Quoted 3 lines
Gr{oetje,eeting}s,
Geert
Signature · 2 lines
Sincerely yours, Mike.
Hi Mike,
Quoted 56 lines · Mike Rapoport
Quoted 38 lines · Geert Uytterhoeven
Quoted 12 lines · Mike Rapoport
From: "Mike Rapoport (Microsoft)" <[email protected]>
kbuild reports the following warning:
arch/sh/kernel/kprobes.c: In function 'kprobe_exceptions_notify':Quoted 1 line
Quoted 1 line
arch/sh/kernel/kprobes.c:412:24: warning: variable 'p' set but not used [-Wunused-but-set-variable]412 | struct kprobe *p = NULL; | ^The variable 'p' is indeed unused since the commit
fa5a24b16f94("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")Quoted 7 lines
Remove that variable along with '
kprobe_opcode_t*addr' which also becomes unused after 'p' is removed.Reported-by kernel test robot <[email protected]>Fixes fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")Signed-off-by Mike Rapoport (Microsoft) <[email protected]>Thanks for your patch!
"p" and "addr" are definitely unused (besides side-effects?), so
Reviewed-by Geert Uytterhoeven <[email protected]>Quoted 12 lines
arch/sh/kernel/kprobes.c +0
-2@@ -404,13 +404,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)int __kprobes kprobe_exceptions_notify (struct notifier_block *self,unsigned long val,void *data) { -struct kprobe *p =NULL ;struct die_args *args = (struct die_args *)data;int ret =NOTIFY_DONE ; -kprobe_opcode_t *addr =NULL ;struct kprobe_ctlblk *kcb =get_kprobe_ctlblk ();Quoted 16 lines
arch/sh/kernel/kprobes.c int __kprobes kprobe_exceptions_notify(struct notifier_block *self, · line 423
Quoted 12 lines
arch/sh/kernel/kprobes.c +0
-2- addr = (kprobe_opcode_t *) (args->regs->pc);if (val ==DIE_TRAP && args->trapnr == (BREAKPOINT_INSTRUCTION &0xff )) {if (!kprobe_running ()) {@@ -421,7 +418,6 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self, ret =NOTIFY_DONE ; } }else { - p =get_kprobe (addr);if ((kcb->kprobe_status ==KPROBE_HIT_SS ) || (kcb->kprobe_status ==KPROBE_REENTER )) {if (post_kprobe_handler (args->regs))I have no idea what this code is supposed to do, and if it actually works. Red flags are that the assigned "p" was never used at all since the inception of this function.
Quoted 3 lines
"p" was used before
fa5a24b16f94("sh/kprobes: Don't call the ->break_handler() in SH kprobes code"), but I can't say I understand that code either :)
Yes, the _variable_ "p" was used before, but not before assigning a different value to it first.
I guess this is where Rust would be helpful? Although C can and does give a warning here, too...
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Mike,
Quoted 46 lines · Mike Rapoport
From: "Mike Rapoport (Microsoft)" <[email protected]>
kbuild reports the following warning:
arch/sh/kernel/kprobes.c: In function 'kprobe_exceptions_notify':Quoted 1 line
Quoted 1 line
arch/sh/kernel/kprobes.c:412:24: warning: variable 'p' set but not used [-Wunused-but-set-variable]412 | struct kprobe *p = NULL; | ^The variable 'p' is indeed unused since the commit
fa5a24b16f94("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")Remove that variable along with '
kprobe_opcode_t*addr' which also becomes unused after 'p' is removed.Reported-by kernel test robot <[email protected]>Fixes fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")Signed-off-by Mike Rapoport (Microsoft) <[email protected]>---
I don't know why the warning poped up only now, the code there didn't change for some time :/
arch/sh/kernel/kprobes.c | 4 ---- 1 file changed, 4 deletions(-)arch/sh/kernel/kprobes.c +0
-3@@ -404,13 +404,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)int __kprobes kprobe_exceptions_notify (struct notifier_block *self,unsigned long val,void *data) { -struct kprobe *p =NULL ;struct die_args *args = (struct die_args *)data;int ret =NOTIFY_DONE ; -kprobe_opcode_t *addr =NULL ;struct kprobe_ctlblk *kcb =get_kprobe_ctlblk (); - addr = (kprobe_opcode_t *) (args->regs->pc);if (val ==DIE_TRAP && args->trapnr == (BREAKPOINT_INSTRUCTION &0xff )) {if (!kprobe_running ()) {
arch/sh/kernel/kprobes.c int __kprobes kprobe_exceptions_notify(struct notifier_block *self, · line 423
Quoted 7 lines
arch/sh/kernel/kprobes.c +0
-1@@ -421,7 +418,6 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self, ret =NOTIFY_DONE ; } }else { - p =get_kprobe (addr);if ((kcb->kprobe_status ==KPROBE_HIT_SS ) || (kcb->kprobe_status ==KPROBE_REENTER )) {if (post_kprobe_handler (args->regs))
Thanks for catching this! Looks good to me!
Adrian
Signature · 4 lines
.''`. John Paul Adrian Glaubitz : :' : Debian Developer `. `' Physicist `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913