Re: [PATCH] Extra error message from update_watchpoint

[Pedro Alves <palves@redhat.com>]

On 10/29/2013 05:38 PM, Andrew Burgess wrote:
> On 29/10/2013 5:19 PM, Eli Zaretskii wrote:
>>> Date: Tue, 29 Oct 2013 16:44:25 +0000
>>> From: "Andrew Burgess" <aburgess@broadcom.com>
>>> cc: "Pedro Alves" <palves@redhat.com>
>>>
>>> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
>>> index 608463d..68b348d 100644
>>> --- a/gdb/breakpoint.c
>>> +++ b/gdb/breakpoint.c
>>> @@ -1805,7 +1805,8 @@ update_watchpoint (struct watchpoint *b, int reparse)
>>>  	  if (b->base.ops->works_in_software_mode (&b->base))
>>>  	    b->base.type = bp_watchpoint;
>>>  	  else
>>> -	    error (_("Software read/access watchpoints not supported."));
>>> +	    error (_("Hardware watchpoint support disabled.  "
>>> +		     "See set/show can-use-hw-watchpoints."));
>>
>> Sorry for chiming in late, but IMO this change is a step backwards:
>> the new warning is much more puzzling than the old one.  The old one
>> at least told what was the problem, the new one looks like entirely
>> unrelated (unless you are privy to GDB internals).
>>
>> How about something like
>>
>>   Cannot set read/access watchpoints without hardware watchpoint support.
> 
> 
> If Pedro is happy then I too am happy, 

;-)  I'm happy.  Eli, it wasn't clear to me from your
suggestion -- do you think the "See set/show can-use-hw-watchpoints."
part should or shouldn't be there?  (the case here isn't that the
target doesn't support these watchpoint types, but that support
has been manually disabled)

> my original issue was for the case where turning H/W watchpoints
> off resulted in the error:
>   Expression cannot be implemented with read/access watchpoint.
> 
> which confused me.  Anything that's been suggested so far is better than
> the original behaviour :)

-- 
Pedro Alves