Re: [patch 1/2] Convert hardware watchpoints to use breakpoint_ops

[Jan Kratochvil <jan.kratochvil@redhat.com>]

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