Re: [rfc target-side break conditions 3/5 v2] GDB-side changes

[Pedro Alves <palves@redhat.com>]

On 02/08/2012 08:56 PM, Luis Gustavo wrote:

>> I'm confused.  How does this (and all similar places) address the issued I
>> pointed out before?  If you're passing one to update_global_location_list
>> when deleting a breakpoint, you're pretty much defeating the whole
>> purpose of the update_global_location_list's argument in the first place.
>>
> 
> Updates will only take place when the user explicitly removes/disables a breakpoint. Functions that are deleting breakpoints (like remove_thread_event_breakpoints) won't cause insertion of any breakpoints since they go through the "delete_breakpoint" path, not the delete_breakpoint_with_update one.

Ah, got it now.  Thanks.  But please:

> +   structures.  If UPDATE is true, proceed to update the list of
> +   locations, otherwise don't update it.  */
>
> -void
> -delete_breakpoint (struct breakpoint *bpt)
> +static void
> +delete_breakpoint_1 (struct breakpoint *bpt, int update)

"update" here is quite confusing, because that's not what is happening.  Or at
least, update is so overloaded that I'm not understanding it the way you are.
The list of locations is still updated/regenerated, and we do delete locations
from the target, but, we don't allow new insertions.  So, could we
s/update/somethingelse/ please?  Even pointing at update_global_location_list's
description of its parameter would be good.

Or maybe, I just had an idea --- I wonder if we made update_global_location_list
still re-insert _only_ the _already inserted_ locations when the
condition changes, then we'd be good.  That is, something like:

update_global_location_list:
...
-  if (breakpoints_always_inserted_mode () && should_insert
-      && (have_live_inferiors ()
-	  || (gdbarch_has_global_breakpoints (target_gdbarch))))
-    insert_breakpoint_locations ();
+  if (breakpoints_always_inserted_mode ()
+      && (have_live_inferiors ()
+	  || (gdbarch_has_global_breakpoints (target_gdbarch))))
+      {
+ 	if (should_insert)
+          insert_breakpoint_locations ();
+        else
+          update_inserted_breakpoint_locations ();
+      }

The problem is that a delete_breakpoint would trigger insertions of all
_other_ breakpoints.  But if we're allow "reinserting" breakpoints that
are _already_ inserted, I think we're fine.

> Is disabling breakpoints also something we would like to do without triggering insertions? If so, i'm inclined to go with the same solution as for deleting user breakpoints.

Maybe, not sure.  Better be safe, I think.

It'd be nice to come up with a better way to solve the initial problem that
led to update_global_location_list having an argument in the first place.  :-/

-- 
Pedro Alves