[PATCHv9,04/14] gdb: the extra_string in a dprintf breakpoint is never nullptr

Message ID 4f415e1a1abea8d9b4e9e8c8837dd7e17097a005.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
  Given the changes in the previous couple of commits, this commit
cleans up some of the asserts and 'if' checks related to the
extra_string within a dprintf breakpoint.

This commit:

  1. Adds some asserts to update_dprintf_command_list about the
  breakpoint type, and that the extra_string is not nullptr,

  2. Given that we know extra_string is not nullptr (this is enforced
  when the breakpoint is created), we can simplify
  code_breakpoint::code_breakpoint -- it no longer needs to check for
  the extra_string is nullptr case,

  3. In dprintf_breakpoint::re_set we can remove the assert (this will
  be checked within update_dprintf_command_list, we can also remove
  the redundant 'if' check.

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

Comments

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

> Given the changes in the previous couple of commits, this commit
> cleans up some of the asserts and 'if' checks related to the
> extra_string within a dprintf breakpoint.
>
> This commit:
>
>   1. Adds some asserts to update_dprintf_command_list about the
>   breakpoint type, and that the extra_string is not nullptr,
>
>   2. Given that we know extra_string is not nullptr (this is enforced
>   when the breakpoint is created), we can simplify
>   code_breakpoint::code_breakpoint -- it no longer needs to check for
>   the extra_string is nullptr case,
>
>   3. In dprintf_breakpoint::re_set we can remove the assert (this will
>   be checked within update_dprintf_command_list, we can also remove
>   the redundant 'if' check.
>
> 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 a7b516ab26c..437dd082fde 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8525,6 +8525,9 @@  bp_loc_is_permanent (struct bp_location *loc)
 static void
 update_dprintf_command_list (struct breakpoint *b)
 {
+  gdb_assert (b->type == bp_dprintf);
+  gdb_assert (b->extra_string != nullptr);
+
   const char *dprintf_args = b->extra_string.get ();
   gdb::unique_xmalloc_ptr<char> printf_line = nullptr;
 
@@ -8698,12 +8701,7 @@  code_breakpoint::code_breakpoint (struct gdbarch *gdbarch_,
       /* 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"));
-	}
+	update_dprintf_command_list (this);
       else if (extra_string != nullptr)
 	error (_("Garbage '%s' at end of command"), extra_string.get ());
     }
@@ -12419,9 +12417,6 @@  dprintf_breakpoint::re_set ()
 {
   re_set_default ();
 
-  /* extra_string should never be non-NULL for dprintf.  */
-  gdb_assert (extra_string != NULL);
-
   /* 1 - connect to target 1, that can run breakpoint commands.
      2 - create a dprintf, which resolves fine.
      3 - disconnect from target 1
@@ -12432,8 +12427,7 @@  dprintf_breakpoint::re_set ()
      answers for target_can_run_breakpoint_commands().
      Given absence of finer grained resetting, we get to do
      it all the time.  */
-  if (extra_string != NULL)
-    update_dprintf_command_list (this);
+  update_dprintf_command_list (this);
 }
 
 /* Implement the "print_recreate" method for dprintf.  */