Doug Evans wrote:
> On Tue, Jun 16, 2009 at 6:31 PM, Aleksandar Ristovski<aristovski@qnx.com> wrote:
>> Aleksandar Ristovski wrote:
>>> Hello,
>>>
>>> This adds support for Z0 and Z1 (and z0 and z1) packets.
>>>
>> I apologize, the patch should be this one. Change log corrected as well.
>>
>> --
>> Aleksandar Ristovski
>> QNX Software Systems
>>
>>
>> ChangeLog:
>>
>> * target.h (insert_breakpoint, remove_breakpoint): New target functions.
>> * server.c (process_serial_event): 'Z' and 'z' packet - add support
>> for [Zz]0 and [Zz]1 - software and hardware breakpoints.
>> * linux-low.c (linux_target_ops): Add new fields.
>> * spu-low.c (spu_target_ops): Likewise.
>> * win32-low.c (win32_target_ops): Likewise.
> 
> Hi.
> 
> Minor nits (fwiw):
> 
> Inserting the insert_breakpoint/remove_breakpoint fields *between* the
> stopped_by_watchpoint/stopped_data_address fields is odd.
> Can they go before insert_watchpoint?
> 
> I think the code in server.c that watches for 0/1/2/3/4 should use a switch.
> 
> The code previously caught invalid values (e.g. type == '5'), but you
> accidentally removed that (IIUC).
> 
> The code in server.c does this:
> 
> +           if (the_target->insert_watchpoint == NULL)
> +             {
> +               /* No watchpoint support or not a watchpoint command;
> +                  unrecognized either way.  */
> +               own_buf[0] = '\0';
> +               res = 1;
> +             }
> 
> and later does this:
> 
> +       if (res == 0)
> +         write_ok (own_buf);
> +       else if (res == 1)
> +         /* Unsupported.  */
> +         own_buf[0] = '\0';
> +       else
> +         write_enn (own_buf);
> 
> How about just setting res = 1 in the first code snippet above?
> [This appears for both 'z' and 'Z' packets.]
> 

Ok, I made changes accordingly. Note that since the code in 
'Z' and 'z' differs only in whether "insert_" or "remove_" 
version of function is called, I merged the two with the 
distinction being made at the calling point.


-- 
Aleksandar Ristovski
QNX Software Systems


ChangeLog:

* target.h (insert_breakpoint, remove_breakpoint): New 
target functions.
* server.c (process_serial_event): 'Z' and 'z' packet - add 
support
for [Zz]0 and [Zz]1 - software and hardware breakpoints.
* linux-low.c (linux_target_ops): Add new fields.
* spu-low.c (spu_target_ops): Likewise.
* win32-low.c (win32_target_ops): Likewise.