Re: [4/9] associate bpstat with location

[Eli Zaretskii <eliz@gnu.org>]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Sat, 8 Sep 2007 00:18:24 +0400
> 
> 	(bpstat_find_breakpoint): Look at bpstat's location's
> 	owner, not at bpstat->breakpoint_at.
> 	(bpstat_find_step_resume_breakpoint): Likewise.
> 	(bpstat_num): Likewise.
> 	(print_it_typical): Likewise.
> 	(print_bp_stop_message): Likewise.
> 	(watchpoint_check): Likewise.
> 	(bpstat_what): Likewise.
> 	(bpstat_get_triggered_catchpoints): Likewise.
> 	(breakpoint_auto_delete): Likewise.
> 	(delete_breakpoint): Likewise.	

A minor stylistic point: could we please avoid the annoying
"Likewise"s?  The canonical way of writing a ChangeLog entry for
several functions with an identical change is this:

	(bpstat_find_breakpoint, bpstat_find_step_resume_breakpoint)
	(bpstat_num, print_it_typical): Look at bpstat's location's
 	owner, not at bpstat->breakpoint_at.

I'm quite sure the GNU coding standards describe this.  (Yes, I know
that our ChangeLog's abuse "Likewise" too much.)

> -  b = (*bsp)->breakpoint_at;
> +  /* We assume we'll never have several bpstats that
> +     correspond to a single breakpoint -- otherwise, 
> +     this function might return the same number more
> +     than once and this will look ugly.  */
> +  b = (*bsp)->breakpoint_at ? (*bsp)->breakpoint_at->owner : NULL;

Is the assumption in the comment above really valid?  I happen to put
several breakpoints on the same line quite a lot (each breakpoint has
a different condition and/or different commands list).

Can we do better, even if it requires to try harder?

>      case bp_access_watchpoint:
>        if (bs->old_val != NULL)     
>  	{
> -	  annotate_watchpoint (bs->breakpoint_at->number);
> +	  annotate_watchpoint (b->number);

Watchpoints also?  Did you make corresponding changes in the code that
sets watchpoints?

Thanks.