Pedro Alves wrote:
> On Tuesday 23 June 2009 16:17:58, Aleksandar Ristovski wrote:
>> Pedro Alves wrote:
>>> On Monday 22 June 2009 20:38:50, Aleksandar Ristovski wrote:
>>>
>>>>> Z0 and Z1 breakpoints also take a 'len' argument, just
>>>>> like Z2-Z4.  You should also pass those down.
>>>>>
>>>>> But, Let's take a step back --- why not just rename the
>>>>> insert_watchpoint|remove_watchpoint functions to insert_point,remove_point,
>>>>> and relax the type checks in server.c:
> 
>>>> But either way is fine with me - just let me know.
>>> I'd prefer the approach I suggested, and worry about splitting
>>> the breakpoints from watchpoints API if/when we actually need it.
>>>
>> Ok, then that version is committed.
> 
> Well, we had never seen "that" version

Ok, to rectify this I am attaching two versions: one if I 
revert the changes I committed and the other is diff to what 
is in now.

 > ... and you bypassed the "rename" suggestion...

I did not do any renaming - I think it is not terribly 
confusing since both in target.h comment and server.c 'Z' 
case it is made very clear that it handles both breakpoints 
and watchpoints (i.e. I don't find it any clearer if it was 
called "insert_point"... it would still require reading the 
comment in target.h)

> 
> Would you care to explain why are watchpoints guarded on
> require_running and breakpoints aren't?  It's not super
> obvious to me.

Both proposed versions now check for require_running for any 
kind of breakpoint.

ChangeLog - diff to what is in now.
* server.c (process_serial_event): Treat all types of
break/watch-points the same.

ChangeLog - diff to what was before my commit:
* server.c (process_serial_event): Add support for Z0 and Z1 
packet.
* target.h: Comment for *_watchpoint to make it clear the 
functions can get types '0' and '1'.

(attached first diff to what is in already and then diff to 
what was before commit).


Thanks,

-- 
Aleksandar Ristovski
QNX Software Systems