Re: RFC: don't set the pspace on ordinary breakpoints

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Tom Tromey wrote:

> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 8c98bef..ec45335 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -1568,6 +1568,9 @@ should_be_inserted (struct bp_location *bl)
>    if (!bl->enabled || bl->shlib_disabled || bl->duplicate)
>      return 0;
>  
> +  if (user_breakpoint_p (bl->owner) && bl->pspace->executing_startup)
> +    return 0;
> +
>    /* This is set for example, when we're attached to the parent of a
>       vfork, and have detached from the child.  The child is running
>       free, and we expect it to do an exec or exit, at which point the
> @@ -5313,8 +5316,7 @@ describe_other_breakpoints (struct gdbarch *gdbarch,
>  	      printf_filtered (" (thread %d)", b->thread);
>  	    printf_filtered ("%s%s ",
>  			     ((b->enable_state == bp_disabled
> -			       || b->enable_state == bp_call_disabled
> -			       || b->enable_state == bp_startup_disabled)
> +			       || b->enable_state == bp_call_disabled)
>  			      ? " (disabled)"
>  			      : b->enable_state == bp_permanent 
>  			      ? " (permanent)"
> @@ -6933,48 +6935,48 @@ enable_watchpoints_after_interactive_call_stop (void)
>  void
>  disable_breakpoints_before_startup (void)
>  {
> -  struct breakpoint *b;
> +  struct bp_location *loc, **locp_tmp;
>    int found = 0;
>  
> -  ALL_BREAKPOINTS (b)
> +  current_program_space->executing_startup = 1;
> +
> +  ALL_BP_LOCATIONS (loc, locp_tmp)
>      {
> -      if (b->pspace != current_program_space)
> +      if (loc->pspace != current_program_space)
>  	continue;
>  
> -      if ((b->type == bp_breakpoint
> -	   || b->type == bp_hardware_breakpoint)
> -	  && breakpoint_enabled (b))
> +      if ((loc->owner->type == bp_breakpoint
> +	   || loc->owner->type == bp_hardware_breakpoint)


Should this now use the same condition as above,
i.e. user_breakpoint_p (b) ?

> +	  && breakpoint_enabled (loc->owner))
>  	{
> -	  b->enable_state = bp_startup_disabled;
>  	  found = 1;
> +	  break;
>  	}
>      }
>  
>    if (found)
>      update_global_location_list (0);

Or maybe rather, since the sole remaining function of that loop
is to sometimes skip the update_global_location_list, which doesn't
appear to have any reason except performance tuning (which is
really unnecessary here as this routine is called extremely rarely)
-- why not just remove the whole loop and just call
update_global_location_list unconditionally?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com