[PATCHv9,05/14] gdb: build dprintf commands just once in code_breakpoint constructor

Message ID 3119a0dcb07fbcb9b0232e92af308653005de7fa.1709651994.git.aburgess@redhat.com
State New
Headers
Series thread-specific breakpoints in just some inferiors |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Andrew Burgess March 5, 2024, 3:21 p.m. UTC
  I noticed in code_breakpoint::code_breakpoint that we are calling
update_dprintf_command_list once for each breakpoint location, when we
really only need to call this once per breakpoint -- the data updated
by this function, the breakpoint command list -- is per breakpoint,
not per breakpoint location.  Calling update_dprintf_command_list
multiple times is just wasted effort, there's no per location error
checking, we don't even pass the current location to the function.

This commit moves the update_dprintf_command_list call outside of the
per-location loop.

There should be no user visible changes after this commit.
---
 gdb/breakpoint.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
  

Comments

Andrew Burgess March 31, 2024, 10:27 a.m. UTC | #1
Andrew Burgess <aburgess@redhat.com> writes:

> I noticed in code_breakpoint::code_breakpoint that we are calling
> update_dprintf_command_list once for each breakpoint location, when we
> really only need to call this once per breakpoint -- the data updated
> by this function, the breakpoint command list -- is per breakpoint,
> not per breakpoint location.  Calling update_dprintf_command_list
> multiple times is just wasted effort, there's no per location error
> checking, we don't even pass the current location to the function.
>
> This commit moves the update_dprintf_command_list call outside of the
> per-location loop.
>
> There should be no user visible changes after this commit.

I've gone ahead and committed this patch.

Thanks,
Andrew
  

Patch

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 437dd082fde..c9023a13d6c 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8697,15 +8697,15 @@  code_breakpoint::code_breakpoint (struct gdbarch *gdbarch_,
       /* Do not set breakpoint locations conditions yet.  As locations
 	 are inserted, they get sorted based on their addresses.  Let
 	 the list stabilize to have reliable location numbers.  */
-
-      /* Dynamic printf requires and uses additional arguments on the
-	 command line, otherwise it's an error.  */
-      if (type == bp_dprintf)
-	update_dprintf_command_list (this);
-      else if (extra_string != nullptr)
-	error (_("Garbage '%s' at end of command"), extra_string.get ());
     }
 
+  /* Dynamic printf requires and uses additional arguments on the
+     command line, otherwise it's an error.  */
+  if (type == bp_dprintf)
+    update_dprintf_command_list (this);
+  else if (extra_string != nullptr)
+    error (_("Garbage '%s' at end of command"), extra_string.get ());
+
   /* The order of the locations is now stable.  Set the location
      condition using the location's number.  */
   int loc_num = 1;