Re: [RFC]: x86 threaded watchpoint support [2/3]

["Eli Zaretskii" <eliz@gnu.org>]

> Date: Fri, 11 Jun 2004 17:33:34 -0400
> From: Jeff Johnston <jjohnstn@redhat.com>
> 
> The most major change is that a check has been added for a
> hardware_watchpoint to ensure that the stopped data address matches
> the watchpoint address.

I don't necessarily object to this change, but could you first explain
why is this needed, while it was never needed before?  (I have my
guess for the answer, but I'd like to hear yours.)

I have also a minor comment about the change itself, see below.

> @@ -2683,45 +2688,100 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
>      if (b->type == bp_watchpoint ||
>  	b->type == bp_hardware_watchpoint)
>        {
> -	char *message = xstrprintf ("Error evaluating expression for watchpoint %d\n",
> +	CORE_ADDR addr;
> +	struct value *v;
> +	int found = 0;
> +
> +	/* If we have a hardware watchpoint, ensure that the address
> +	   being watched caused the trap event.  */
> +	if (b->type == bp_hardware_watchpoint)
> +	  {
> +	    addr = target_stopped_data_address ();
> +	    if (addr == 0)
> +	      {
> +	        /* Don't stop.  */
> +	        bs->print_it = print_it_noop;
> +	        bs->stop = 0;
> +	        continue;
> +	      }
> +	    for (v = b->val_chain; v; v = v->next)

It looks to me that this change makes the bp_hardware_watchpoint case
exactly identical to bp_read_watchpoint and bp_access_watchpoint, is
that right?  If so, why not add bp_hardware_watchpoint to the if
clause that handles read and access watchpoints, and leave only
bp_watchpoint alone with the current code?