Re: functions that delete breakpoints should not insert bp locations (was: Fix execl.exp sporadic failures)

[Vladimir Prus <vladimir@codesourcery.com>]

Pedro Alves wrote:

> static void
> -update_global_location_list (void)
> +update_global_location_list (int inserting)

There's no comment for the new parameter. Also, the parameter tells
the function to do, or not do something, so "-ing" sounds awkward.
How about 'should_insert' or 'suppress_insert'?

> {
> struct breakpoint *b;
> struct bp_location **next = &bp_location_chain;
> @@ -7132,7 +7123,11 @@ update_global_location_list (void)
> check_duplicates (b);
> }
> 
> -  if (always_inserted_mode && target_has_execution)
> +  /* If we get here due to deleting a breakpoint, don't try to insert
> +     locations.  

This does not tell how 'inserting' is related to 'deleting a breakpoint'.

> This helps cases like: target_detach deleting a 
> +     breakpoint before detaching causing all other breakpoints to be
> +     inserted.  */


I'd suggest to remove this comment, and add a comment before function saying this:

/* If SUPPRESS_INSERT is true, do not insert any breakpoint locations into target,
   only remove already-inserted locations that no longer should be inserted.
   This behaviour is useful during tear-down -- when the target no longer is
   capable of inserting breakpoints, and we're removing internal GDB breakpoints,
   we don't want to try inserting other breakpoints into already-gone target.  */

As we've already discussed, I think the ideal solution is to mark the target is
'going down' and suppress insertion of breakpoints if the target is in that state.
Your patch, however, should be almost as good, so I don't have any objections.
Thanks for addressing this issue!

- Volodya