This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] gdbserver: Add support for Z0/Z1 packets
- From: Doug Evans <dje at google dot com>
- To: Aleksandar Ristovski <aristovski at qnx dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Fri, 19 Jun 2009 00:08:38 -0700
- Subject: Re: [patch] gdbserver: Add support for Z0/Z1 packets
- References: <h19g9r$sht$2@ger.gmane.org> <h19h0q$u4m$1@ger.gmane.org>
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.]