This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch 1/2] Convert hardware watchpoints to use breakpoint_ops
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: Thiago Jung Bauermann <bauerman at br dot ibm dot com>
- Cc: Pedro Alves <pedro at codesourcery dot com>, gdb-patches at sourceware dot org
- Date: Tue, 16 Nov 2010 05:06:25 +0100
- Subject: Re: [patch 1/2] Convert hardware watchpoints to use breakpoint_ops
- References: <1282074071.2606.702.camel@hactar> <201010161843.43062.pedro@codesourcery.com> <1287534691.2686.17.camel@hactar>
On Wed, 20 Oct 2010 02:31:31 +0200, Thiago Jung Bauermann wrote:
> -static void
> -insert_catch_fork (struct breakpoint *b)
> +static int
> +insert_catch_fork (struct bp_location *b)
Such variables (across the whole patch) should be really renamed when changing
its type.
I understand there are already cases of such incorrect naming in GDB but we
should not make it worse.
Also were these functions intended per-breakpoint or per-bp_location?
It looks to me currently they are used only for single-location breakpoint so
no one knows. (I guess they were meant for breakpoint.)
> -struct breakpoint_ops
> +struct breakpoint_ops
> {
> - /* Insert the breakpoint or activate the catchpoint. Should raise
> - an exception if the operation failed. */
> - void (*insert) (struct breakpoint *);
> + /* Insert the breakpoint or watchpoint or activate the catchpoint.
> + Return 0 for success, 1 if the breakpoint, watchpoint or catchpoint
> + type is not supported, -1 for failure. */
> + int (*insert) (struct bp_location *);
>
> /* Remove the breakpoint/catchpoint that was previously inserted
> - with the "insert" method above. Return non-zero if the operation
> - succeeded. */
> - int (*remove) (struct breakpoint *);
> + with the "insert" method above. Return 0 for success, 1 if the
> + breakpoint, watchpoint or catchpoint type is not supported,
> + -1 for failure. */
> + int (*remove) (struct bp_location *);
At least rename it to insert_bploc (or insert_location etc.). This will need
to be cleaned up with the regular breakpoints/watchpoints conversion to
breakpoint_ops.
Thanks,
Jan