This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3 1/5] Add the target_ops needed for software breakpoints in GDBServer.
- From: Pedro Alves <palves at redhat dot com>
- To: Antoine Tremblay <antoine dot tremblay at ericsson dot com>, gdb-patches at sourceware dot org
- Date: Wed, 21 Oct 2015 12:18:41 +0100
- Subject: Re: [PATCH v3 1/5] Add the target_ops needed for software breakpoints in GDBServer.
- Authentication-results: sourceware.org; auth=none
- References: <1445359685-2589-1-git-send-email-antoine dot tremblay at ericsson dot com> <1445359685-2589-2-git-send-email-antoine dot tremblay at ericsson dot com>
On 10/20/2015 05:48 PM, Antoine Tremblay wrote:
> --- a/gdb/gdbserver/linux-low.h
> +++ b/gdb/gdbserver/linux-low.h
> @@ -141,8 +141,17 @@ struct linux_target_ops
>
> CORE_ADDR (*get_pc) (struct regcache *regcache);
> void (*set_pc) (struct regcache *regcache, CORE_ADDR newpc);
> - const unsigned char *breakpoint;
> - int breakpoint_len;
> +
> + /* Return the breakpoint kind for this target based on PC. The PCPTR is
> + adjusted to the real memory location in case a flag (e.g., the Thumb bit on
> + ARM) was present in the PC. */
Spurious double space before "was present".
> + int (*breakpoint_kind_from_pc) (CORE_ADDR *pcptr);
> +
> + /* Return the software breakpoint from KIND. KIND can have target
> + specific meaning like the z0 kind parameter.
It's more usual to talk in terms of the "insert" packet. That is,
write uppercase Z0.
> --- a/gdb/gdbserver/target.h
> +++ b/gdb/gdbserver/target.h
> @@ -441,6 +441,16 @@ struct target_ops
> readlink(2). */
> ssize_t (*multifs_readlink) (int pid, const char *filename,
> char *buf, size_t bufsiz);
> +
> + /* Return the breakpoint kind for this target based on PC. The PCPTR is
> + adjusted to the real memory location in case a flag (e.g., the Thumb bit on
> + ARM) was present in the PC. */
> + int (*breakpoint_kind_from_pc) (CORE_ADDR *pcptr);
> +
> + /* Return the software breakpoint from KIND. KIND can have target
> + specific meaning like the z0 kind parameter.
> + SIZE is set to the software breakpoint's length in memory. */
> + const gdb_byte *(*sw_breakpoint_from_kind) (int kind, int *size);
Ditto.
I think the linux-low.h comments should just say "See target.h for details."
or some such, avoiding the duplication (and the forgetting to update
linux-low.h when target.h next changes).
Otherwise LGTM.
Thanks,
Pedro Alves