[PATCH] kdb: Fix breakpoint enable to be silent if already enabled

[PATCH] kdb: Fix breakpoint enable to be silent if already enabled

[Nir Lichtman]

Fix the breakpoint enable command (be) to a logic that is inline with the
breakpoint disable command (bd) in which if the breakpoint is already in
an enabled state, do not print the message of enabled again to the user.

Also a small nit fix of the new line in a separate print.

Signed-off-by: Nir Lichtman <nir@lichtman.org>

---
 kernel/debug/kdb/kdb_bp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/debug/kdb/kdb_bp.c b/kernel/debug/kdb/kdb_bp.c
index 372025cf1ca3..c0c2072f5452 100644
--- a/kernel/debug/kdb/kdb_bp.c
+++ b/kernel/debug/kdb/kdb_bp.c
@@ -460,13 +460,15 @@ static int kdb_bc(int argc, const char **argv)
 
 			break;
 		case KDBCMD_BE:
+			if (bp->bp_enabled)
+				break;
+
 			bp->bp_enabled = 1;
 
 			kdb_printf("Breakpoint %d at "
-				   kdb_bfd_vma_fmt " enabled",
+				   kdb_bfd_vma_fmt " enabled\n",
 				   i, bp->bp_addr);
 
-			kdb_printf("\n");
 			break;
 		case KDBCMD_BD:
 			if (!bp->bp_enabled)
-- 
2.39.2

Re: [PATCH] kdb: Fix breakpoint enable to be silent if already enabled

[Doug Anderson]

Hi,

On Sun, Oct 27, 2024 at 1:47 PM Nir Lichtman <nir@lichtman.org> wrote:
>
> Fix the breakpoint enable command (be) to a logic that is inline with the
> breakpoint disable command (bd) in which if the breakpoint is already in
> an enabled state, do not print the message of enabled again to the user.
>
> Also a small nit fix of the new line in a separate print.
>
> Signed-off-by: Nir Lichtman <nir@lichtman.org>
>
> ---
>  kernel/debug/kdb/kdb_bp.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

I don't ever use these commands (if I'm setting breakpoints them I'm
using kgdb, not kdb), but this seems OK to me in general. I'm a little
curious why you don't also make the "breakpoint clear" command
consistent.

-Doug

Re: [PATCH] kdb: Fix breakpoint enable to be silent if already enabled

[Nir Lichtman]

On Mon, Oct 28, 2024 at 11:38:00AM -0700, Doug Anderson wrote:
> 
> On Sun, Oct 27, 2024 at 1:47 PM Nir Lichtman <nir@lichtman.org> wrote:
> >
> > Fix the breakpoint enable command (be) to a logic that is inline with the
> > breakpoint disable command (bd) in which if the breakpoint is already in
> > an enabled state, do not print the message of enabled again to the user.
> >
> 
> I don't ever use these commands (if I'm setting breakpoints them I'm
> using kgdb, not kdb), but this seems OK to me in general. I'm a little
> curious why you don't also make the "breakpoint clear" command
> consistent.

That's a good point I haven't looked at the bc logic,
will fix that as well and send a v2.

Thanks for the review,
Nir

> 
> -Doug

Re: [PATCH] kdb: Fix breakpoint enable to be silent if already enabled

[Nir Lichtman]

On Mon, Oct 28, 2024 at 11:38:00AM -0700, Doug Anderson wrote:
> On Sun, Oct 27, 2024 at 1:47 PM Nir Lichtman <nir@lichtman.org> wrote:
> >
> > Fix the breakpoint enable command (be) to a logic that is inline with the
> > breakpoint disable command (bd) in which if the breakpoint is already in
> > an enabled state, do not print the message of enabled again to the user.
> 
> I don't ever use these commands (if I'm setting breakpoints them I'm
> using kgdb, not kdb), but this seems OK to me in general. I'm a little
> curious why you don't also make the "breakpoint clear" command
> consistent.
> 

After looking deeper into this, reason this is not applicable for the "bc"
command is that after clearing the breakpoint, upon trying to clear it again,
an error will occur that says the breakpoint doesn't exist anymore hence this
logic is not applicable for "bc" which unlike "be"/"bd" completely clear out the bp

Thanks,
Nir

> -Doug

Re: [PATCH] kdb: Fix breakpoint enable to be silent if already enabled

[Daniel Thompson]

On Sun, 27 Oct 2024 20:47:29 +0000, Nir Lichtman wrote:
> Fix the breakpoint enable command (be) to a logic that is inline with the
> breakpoint disable command (bd) in which if the breakpoint is already in
> an enabled state, do not print the message of enabled again to the user.
> 
> Also a small nit fix of the new line in a separate print.
> 
> 
> [...]

Applied, thanks!

[1/1] kdb: Fix breakpoint enable to be silent if already enabled
      commit: 1d0de1fbfcbf480b785f3e57a098977a5f6722c6

Best regards,
-- 
Daniel Thompson <daniel.thompson@linaro.org>