clear_breakpoint cleanup

clear_breakpoint cleanup

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 322 bytes --]


Now, clear_breakpoint directly removes breakpoints from
global breakpoint list, even though delete_breakpoint
can do it just fine. This is not causing any bugs, but
for code simplicity sake, I'd like to reduce the amount
of code that accesses breakpoint list. The following
patch causes no regressions. OK?

- Volodya




[-- Attachment #2: clear_breakpoint.diff --]
[-- Type: text/x-diff, Size: 5737 bytes --]

commit 11627d82afe62aa41c71310e946050dc303f7d42
Author: Vladimir Prus <vladimir@codesourcery.com>
Date:   Wed Nov 14 19:14:21 2007 +0300

    Prevent clear_command from directly modifying breakpoint list.
    
    	gdb/
    	* Makefile.in (breakpoint_h): Update dependency.
    	* breakpoint.c (clear_command): Do not remove
    	breakpoints from breakpoint_chain.  Collect breakpoints
    	to delete in a vector.
    	* breakpoint.h (breakpoint_p): New typedef for pointer to
    	breakpoint.  Register vector of breakpoint_p.
    
    	testsuite/
    	* gdb.base/ending-run.exp: Expect the list of cleared
    	breakpoint to come in natural order, not the reversed one.

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 79efa0d..3effc02 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -678,7 +678,7 @@ ax_h = ax.h $(doublest_h)
 bcache_h = bcache.h
 bfd_target_h = bfd-target.h
 block_h = block.h
-breakpoint_h = breakpoint.h $(frame_h) $(value_h) $(gdb_events_h)
+breakpoint_h = breakpoint.h $(frame_h) $(value_h) $(gdb_events_h) $(vec_h)
 bsd_kvm_h = bsd-kvm.h
 bsd_uthread_h = bsd-uthread.h
 buildsym_h = buildsym.h
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 5412e78..3dae38f 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6878,7 +6878,9 @@ tcatch_command (char *arg, int from_tty)
 static void
 clear_command (char *arg, int from_tty)
 {
-  struct breakpoint *b, *tmp, *prev, *found;
+  struct breakpoint *b;
+  VEC(breakpoint_p) *found = 0;
+  int ix;
   int default_match;
   struct symtabs_and_lines sals;
   struct symtab_and_line sal;
@@ -6945,11 +6947,10 @@ clear_command (char *arg, int from_tty)
          1              0             <can't happen> */
 
       sal = sals.sals[i];
-      prev = NULL;
 
-      /* Find all matching breakpoints, remove them from the
-	 breakpoint chain, and add them to the 'found' chain.  */
-      ALL_BREAKPOINTS_SAFE (b, tmp)
+      /* Find all matching breakpoints and add them to
+	 'found'.  */
+      ALL_BREAKPOINTS (b)
 	{
 	  int match = 0;
 	  /* Are we going to delete b? */
@@ -6980,30 +6981,11 @@ clear_command (char *arg, int from_tty)
 	    }
 
 	  if (match)
-	    {
-	      /* Remove it from breakpoint_chain...  */
-	      if (b == breakpoint_chain)
-		{
-		  /* b is at the head of the list */
-		  breakpoint_chain = b->next;
-		}
-	      else
-		{
-		  prev->next = b->next;
-		}
-	      /* And add it to 'found' chain.  */
-	      b->next = found;
-	      found = b;
-	    }
-	  else
-	    {
-	      /* Keep b, and keep a pointer to it.  */
-	      prev = b;
-	    }
+	    VEC_safe_push(breakpoint_p, found, b);
 	}
     }
   /* Now go thru the 'found' chain and delete them.  */
-  if (found == 0)
+  if (VEC_empty(breakpoint_p, found))
     {
       if (arg)
 	error (_("No breakpoint at %s."), arg);
@@ -7011,23 +6993,22 @@ clear_command (char *arg, int from_tty)
 	error (_("No breakpoint at this line."));
     }
 
-  if (found->next)
+  if (VEC_length(breakpoint_p, found) > 1)
     from_tty = 1;		/* Always report if deleted more than one */
   if (from_tty)
     {
-      if (!found->next)
+      if (VEC_length(breakpoint_p, found) == 1)
 	printf_unfiltered (_("Deleted breakpoint "));
       else
 	printf_unfiltered (_("Deleted breakpoints "));
     }
   breakpoints_changed ();
-  while (found)
+
+  for (ix = 0; VEC_iterate(breakpoint_p, found, ix, b); ix++)
     {
       if (from_tty)
-	printf_unfiltered ("%d ", found->number);
-      tmp = found->next;
-      delete_breakpoint (found);
-      found = tmp;
+	printf_unfiltered ("%d ", b->number);
+      delete_breakpoint (b);
     }
   if (from_tty)
     putchar_unfiltered ('\n');
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 08a10ab..19b880c 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -22,6 +22,7 @@
 
 #include "frame.h"
 #include "value.h"
+#include "vec.h"
 
 #include "gdb-events.h"
 
@@ -457,6 +458,9 @@ struct breakpoint
        the condition in.  */
     int condition_not_parsed;
   };
+
+typedef struct breakpoint *breakpoint_p;
+DEF_VEC_P(breakpoint_p);
 \f
 /* The following stuff is an abstract data type "bpstat" ("breakpoint
    status").  This provides the ability to determine whether we have
diff --git a/gdb/testsuite/gdb.base/ending-run.exp b/gdb/testsuite/gdb.base/ending-run.exp
index 3a35e08..82e4efe 100644
--- a/gdb/testsuite/gdb.base/ending-run.exp
+++ b/gdb/testsuite/gdb.base/ending-run.exp
@@ -69,7 +69,7 @@ gdb_test "b ending-run.c:31" ".*Breakpoint 3.*ending-run.c, line 31.*"
 gdb_run_cmd
 gdb_test "" ".*Breakpoint.*1.*callee.*13.*" "run"
 
-gdb_test "cle" ".*Deleted breakpoints 2 1.*" "clear worked"
+gdb_test "cle" ".*Deleted breakpoints 1 2.*" "clear worked"
 send_gdb "i b\n"
 gdb_expect {
     -re ".* breakpoint .* breakpoint .*$gdb_prompt $" { 
@@ -95,7 +95,7 @@ gdb_expect {
     -re ".*address (0x\[0-9a-fA-F]*).*$gdb_prompt $" {
         set line_eight $expect_out(1,string)
         gdb_test "b 13" ".*Breakpoint.*6.*"
-        gdb_test "cle *$line_eight" ".*Deleted breakpoints 6 4.*" "Clear 2 by address"
+        gdb_test "cle *$line_eight" ".*Deleted breakpoints 4 6.*" "Clear 2 by address"
     }
     -re ".*$gdb_prompt $" {
         fail "need to fix test for new compile outcome"
@@ -109,7 +109,7 @@ gdb_expect {
         gdb_test "b ending-run.c:14" ".*Breakpoint 7.*ending-run.c, line 14.*"
         gdb_test "b *$line_nine" ".*Note.*also.*Breakpoint 8.*" "Breakpoint 7 at *ending-run.c:14"
         gdb_test "c" ".*Breakpoint.*7.*callee.*14.*"
-        gdb_test "cle" ".*Deleted breakpoints 8 7.*" "Clear 2 by default"
+        gdb_test "cle" ".*Deleted breakpoints 7 8.*" "Clear 2 by default"
     }
     -re ".*$gdb_prompt $" {
         fail "need to fix test for new compile outcome"

Re: clear_breakpoint cleanup

[Jim Blandy]

Vladimir Prus <vladimir at codesourcery.com> writes:
> Now, clear_breakpoint directly removes breakpoints from
> global breakpoint list, even though delete_breakpoint
> can do it just fine. This is not causing any bugs, but
> for code simplicity sake, I'd like to reduce the amount
> of code that accesses breakpoint list. The following
> patch causes no regressions. OK?

This looks good to me.