Messages in this thread | | | Subject | Re: [PATCH 1/2] powerpc/kprobes: Remove redundant code | From | Christophe Leroy <> | Date | Wed, 19 Feb 2020 08:48:09 +0100 |
| |
Le 18/02/2020 à 15:39, Naveen N. Rao a écrit : > Christophe Leroy wrote: >> At the time being we have something like >> >> if (something) { >> p = get(); >> if (p) { >> if (something_wrong) >> goto out; >> ... >> return; >> } else if (a != b) { >> if (some_error) >> goto out; >> ... >> } >> goto out; >> } >> p = get(); >> if (!p) { >> if (a != b) { >> if (some_error) >> goto out; >> ... >> } >> goto out; >> } >> >> This is similar to >> >> p = get(); >> if (something) { >> if (p) { >> if (something_wrong) >> goto out; >> ... >> return; >> } >> } >> if (!p) { >> if (a != b) { >> if (some_error) >> goto out; >> ... >> } >> goto out; >> } >> >> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> >> --- >> arch/powerpc/kernel/kprobes.c | 15 +-------------- >> 1 file changed, 1 insertion(+), 14 deletions(-) > > Good cleanup, thanks. > >> >> diff --git a/arch/powerpc/kernel/kprobes.c >> b/arch/powerpc/kernel/kprobes.c >> index f8b848aa65bd..7a925eb76ec0 100644 >> --- a/arch/powerpc/kernel/kprobes.c >> +++ b/arch/powerpc/kernel/kprobes.c >> @@ -276,8 +276,8 @@ int kprobe_handler(struct pt_regs *regs) >> kcb = get_kprobe_ctlblk(); >> >> /* Check we're not actually recursing */ >> + p = get_kprobe(addr); >> if (kprobe_running()) { >> - p = get_kprobe(addr); >> if (p) { >> kprobe_opcode_t insn = *p->ainsn.insn; >> if (kcb->kprobe_status == KPROBE_HIT_SS && >> @@ -308,22 +308,9 @@ int kprobe_handler(struct pt_regs *regs) >> } >> prepare_singlestep(p, regs); >> return 1; >> - } else if (*addr != BREAKPOINT_INSTRUCTION) { >> - /* If trap variant, then it belongs not to us */ >> - kprobe_opcode_t cur_insn = *addr; >> - >> - if (is_trap(cur_insn)) >> - goto no_kprobe; >> - /* The breakpoint instruction was removed by >> - * another cpu right after we hit, no further >> - * handling of this interrupt is appropriate >> - */ >> - ret = 1; >> } >> - goto no_kprobe; > > A minot nit -- removing the above goto makes a slight change to the > logic. But, see my comments for the next patch.
All legs of the (p) case are have either a return or a goto, so that goto no_kprobe is limited to the !p case, we have to fall_through now.
Christophe
| |