[PATCHv4 04/10] gdb: build dprintf commands just once in code_breakpoint constructor

Andrew Burgess via Gdb-patches
2023-08-23T16:00:06Z

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. I have also changes the 'if' that handles the case
where the extra_string (which holds the format/args) is empty. I
don't believe that this situation can ever arise -- and if it does we
should be catching it earlier and throwing an error at that point.

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

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 86225ef82fa..1cf1f4468a1 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8709,19 +8709,17 @@ 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)
- {
- if (extra_string != nullptr)
- update_dprintf_command_list (this);
- else
- error (_("Format string required"));
- }
- 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)
+ {
+ gdb_assert (extra_string != nullptr);
+ 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. */
--
2.25.4