[obvious] Remove unused argument in insert_bp_location

[obvious] Remove unused argument in insert_bp_location

[Thiago Jung Bauermann]

Hi,

I noticed that disabled_breakpoints is not used. It's both an input and
output parameter. All callers set it to zero, and none of them checks
its value on return.

Committed the following (there were no regressions on i386-linux).
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


2011-07-31  Thiago Jung Bauermann  <bauerman@br.ibm.com>

	* breakpoint.c (insert_bp_location): Remove disabled_breaks
	argument.  Update callers.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 00fe748..edfa661 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1573,14 +1573,13 @@ should_be_inserted (struct bp_location *bl)
 
 /* Insert a low-level "breakpoint" of some type.  BL is the breakpoint
    location.  Any error messages are printed to TMP_ERROR_STREAM; and
-   DISABLED_BREAKS, and HW_BREAKPOINT_ERROR are used to report problems.
+   HW_BREAKPOINT_ERROR is used to report problems.
 
    NOTE drow/2003-09-09: This routine could be broken down to an
    object-style method for each breakpoint or catchpoint type.  */
 static int
 insert_bp_location (struct bp_location *bl,
 		    struct ui_file *tmp_error_stream,
-		    int *disabled_breaks,
 		    int *hw_breakpoint_error)
 {
   int val = 0;
@@ -1714,16 +1713,12 @@ insert_bp_location (struct bp_location *bl,
 	      val = 0;
 	      bl->shlib_disabled = 1;
 	      observer_notify_breakpoint_modified (bl->owner);
-	      if (!*disabled_breaks)
-		{
-		  fprintf_unfiltered (tmp_error_stream, 
-				      "Cannot insert breakpoint %d.\n", 
-				      bl->owner->number);
-		  fprintf_unfiltered (tmp_error_stream, 
-				      "Temporarily disabling shared "
-				      "library breakpoints:\n");
-		}
-	      *disabled_breaks = 1;
+	      fprintf_unfiltered (tmp_error_stream, 
+			          "Cannot insert breakpoint %d.\n", 
+				  bl->owner->number);
+	      fprintf_unfiltered (tmp_error_stream, 
+				  "Temporarily disabling shared "
+				  "library breakpoints:\n");
 	      fprintf_unfiltered (tmp_error_stream,
 				  "breakpoint #%d\n", bl->owner->number);
 	    }
@@ -1914,7 +1909,6 @@ insert_breakpoint_locations (void)
   struct bp_location *bl, **blp_tmp;
   int error = 0;
   int val = 0;
-  int disabled_breaks = 0;
   int hw_breakpoint_error = 0;
 
   struct ui_file *tmp_error_stream = mem_fileopen ();
@@ -1948,8 +1942,8 @@ insert_breakpoint_locations (void)
 	  && ptid_equal (inferior_ptid, null_ptid))
 	continue;
 
-      val = insert_bp_location (bl, tmp_error_stream, &disabled_breaks,
-				    &hw_breakpoint_error);
+      val = insert_bp_location (bl, tmp_error_stream,
+				&hw_breakpoint_error);
       if (val)
 	error = val;
     }
@@ -2052,7 +2046,7 @@ reattach_breakpoints (int pid)
   struct bp_location *bl, **blp_tmp;
   int val;
   struct ui_file *tmp_error_stream;
-  int dummy1 = 0, dummy2 = 0;
+  int dummy = 0;
   struct inferior *inf;
   struct thread_info *tp;
 
@@ -2076,7 +2070,7 @@ reattach_breakpoints (int pid)
     if (bl->inserted)
       {
 	bl->inserted = 0;
-	val = insert_bp_location (bl, tmp_error_stream, &dummy1, &dummy2);
+	val = insert_bp_location (bl, tmp_error_stream, &dummy);
 	if (val != 0)
 	  {
 	    do_cleanups (old_chain);

Re: [obvious] Remove unused argument in insert_bp_location

[Tom Tromey]

>>>>> "Thiago" == Thiago Jung Bauermann <bauerman@br.ibm.com> writes:

Thiago> I noticed that disabled_breakpoints is not used. It's both an input and
Thiago> output parameter. All callers set it to zero, and none of them checks
Thiago> its value on return.

Thiago> 2011-07-31  Thiago Jung Bauermann  <bauerman@br.ibm.com>
Thiago> 	* breakpoint.c (insert_bp_location): Remove disabled_breaks
Thiago> 	argument.  Update callers.

I don't think this patch is really equivalent to the previous code.

To me it seems that *disabled_breaks was being used as a persistent flag.
That is, the caller doesn't check its value, but subsequent calls to
insert_bp_location do -- the value is not reset in the ALL_BP_LOCATIONS
loop in insert_breakpoint_locations.

Tom

Re: [obvious] Remove unused argument in insert_bp_location

[Thiago Jung Bauermann]

On Mon, 2011-08-01 at 08:41 -0600, Tom Tromey wrote:
> >>>>> "Thiago" == Thiago Jung Bauermann <bauerman@br.ibm.com> writes:
> 
> Thiago> I noticed that disabled_breakpoints is not used. It's both an input and
> Thiago> output parameter. All callers set it to zero, and none of them checks
> Thiago> its value on return.
> 
> Thiago> 2011-07-31  Thiago Jung Bauermann  <bauerman@br.ibm.com>
> Thiago> 	* breakpoint.c (insert_bp_location): Remove disabled_breaks
> Thiago> 	argument.  Update callers.
> 
> I don't think this patch is really equivalent to the previous code.
> 
> To me it seems that *disabled_breaks was being used as a persistent flag.
> That is, the caller doesn't check its value, but subsequent calls to
> insert_bp_location do -- the value is not reset in the ALL_BP_LOCATIONS
> loop in insert_breakpoint_locations.

Oh, you're right. It prevents those warnings from being shown for every
bp_location in a shared library. I'll revert the patch and extend the
documentation of insert_bp_location.

That's one of the problems of hacking on a Sunday. Thanks for checking.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [obvious] Remove unused argument in insert_bp_location

[Thiago Jung Bauermann]

On Mon, 2011-08-01 at 15:34 -0300, Thiago Jung Bauermann wrote:
> On Mon, 2011-08-01 at 08:41 -0600, Tom Tromey wrote:
> > I don't think this patch is really equivalent to the previous code.
> > 
> > To me it seems that *disabled_breaks was being used as a persistent flag.
> > That is, the caller doesn't check its value, but subsequent calls to
> > insert_bp_location do -- the value is not reset in the ALL_BP_LOCATIONS
> > loop in insert_breakpoint_locations.
> 
> Oh, you're right. It prevents those warnings from being shown for every
> bp_location in a shared library. I'll revert the patch and extend the
> documentation of insert_bp_location.
> 
> That's one of the problems of hacking on a Sunday. Thanks for checking.

Just reverted it.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


2011-08-01  Thiago Jung Bauermann  <bauerman@br.ibm.com>

	Revert:
	2011-07-31  Thiago Jung Bauermann  <bauerman@br.ibm.com>
	* breakpoint.c (insert_bp_location): Remove disabled_breaks
	argument.  Update callers.

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.605
diff -u -r1.605 breakpoint.c
--- breakpoint.c	31 Jul 2011 20:31:16 -0000	1.605
+++ breakpoint.c	1 Aug 2011 18:41:34 -0000
@@ -1573,7 +1573,7 @@
 
 /* Insert a low-level "breakpoint" of some type.  BL is the breakpoint
    location.  Any error messages are printed to TMP_ERROR_STREAM; and
-   HW_BREAKPOINT_ERROR is used to report problems.
+   DISABLED_BREAKS, and HW_BREAKPOINT_ERROR are used to report problems.
    Returns 0 for success, 1 if the bp_location type is not supported or
    -1 for failure.
 
@@ -1582,6 +1582,7 @@
 static int
 insert_bp_location (struct bp_location *bl,
 		    struct ui_file *tmp_error_stream,
+		    int *disabled_breaks,
 		    int *hw_breakpoint_error)
 {
   int val = 0;
@@ -1715,12 +1716,16 @@
 	      val = 0;
 	      bl->shlib_disabled = 1;
 	      observer_notify_breakpoint_modified (bl->owner);
-	      fprintf_unfiltered (tmp_error_stream, 
-			          "Cannot insert breakpoint %d.\n", 
-				  bl->owner->number);
-	      fprintf_unfiltered (tmp_error_stream, 
-				  "Temporarily disabling shared "
-				  "library breakpoints:\n");
+	      if (!*disabled_breaks)
+		{
+		  fprintf_unfiltered (tmp_error_stream, 
+				      "Cannot insert breakpoint %d.\n", 
+				      bl->owner->number);
+		  fprintf_unfiltered (tmp_error_stream, 
+				      "Temporarily disabling shared "
+				      "library breakpoints:\n");
+		}
+	      *disabled_breaks = 1;
 	      fprintf_unfiltered (tmp_error_stream,
 				  "breakpoint #%d\n", bl->owner->number);
 	    }
@@ -1908,6 +1913,7 @@
   struct bp_location *bl, **blp_tmp;
   int error = 0;
   int val = 0;
+  int disabled_breaks = 0;
   int hw_breakpoint_error = 0;
 
   struct ui_file *tmp_error_stream = mem_fileopen ();
@@ -1941,8 +1947,8 @@
 	  && ptid_equal (inferior_ptid, null_ptid))
 	continue;
 
-      val = insert_bp_location (bl, tmp_error_stream,
-				&hw_breakpoint_error);
+      val = insert_bp_location (bl, tmp_error_stream, &disabled_breaks,
+				    &hw_breakpoint_error);
       if (val)
 	error = val;
     }
@@ -2049,7 +2055,7 @@
   struct bp_location *bl, **blp_tmp;
   int val;
   struct ui_file *tmp_error_stream;
-  int dummy = 0;
+  int dummy1 = 0, dummy2 = 0;
   struct inferior *inf;
   struct thread_info *tp;
 
@@ -2073,7 +2079,7 @@
     if (bl->inserted)
       {
 	bl->inserted = 0;
-	val = insert_bp_location (bl, tmp_error_stream, &dummy);
+	val = insert_bp_location (bl, tmp_error_stream, &dummy1, &dummy2);
 	if (val != 0)
 	  {
 	    do_cleanups (old_chain);