Re: Save the length of inserted breakpoints

[Eli Zaretskii <eliz@gnu.org>]

> Date: Mon, 17 Apr 2006 15:08:08 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: msnyder@redhat.com, gdb-patches@sourceware.org
> 
> On the documentation front, I am quite mortified; I wrote this
> ChangeLog entry at least twice for earlier versions of the patch:
> 
>         * gdbint.texinfo (Watchpoints, Target Conditionals): Update insert
>         and remove breakpoint prototypes.
> 
> While they often use the underlying watchpoint registers, hardware
> breakpoints are breakpoints.  I've moved these to the Breakpoints
> section, and added some more bits there.  Eli, could you take a look
> at that?

Done, comments below.

>                                                @var{bp_tgt} is updated
> +to contain other information about the breakpoint on output:
> +@code{placed_address} may be updated if the breakpoint was placed at a
> +related address; @code{shadow_contents} is the real contents of the bytes
> +where the breakpoint has been inserted, if reading memory would return
> +the breakpoint instead of the underlying memory; @code{shadow_len} is the
> +length of memory cached in @code{shadow_contents}, if any; and
> +@code{placed_size} is optionally set and used by the target, if
> +it could differ from @code{shadow_len}.

This paragraph uses names of fields of the bp_target_info struct, but
it doesn't mention the name of the struct, and doesn't say that those
are its fields.  I suggest to modify the first sentence above like
this:

  Fields of @code{struct bp_target_info} pointed to by @var{bp_tgt}
  are updated to contain other information about the breakpoint on
  output: @code{@var{bp_tgt}->placed_address} may be updated if the
  breakpoint was placed ...

etc., you get the idea.  Alternatively, something like "the field
@code{placed_address} may be updated ..." is also okay.

> +@item i386_insert_hw_breakpoint (@var{bp_tgt})
> +@itemx i386_remove_hw_breakpoint (@var{bp_tgt})

This should mention the type of the struct:

 @item i386_insert_hw_breakpoint (struct bp_target_info *@var{bp_tgt})

Alternatively, mention the name of the struct in the text.

> +@item MEMORY_INSERT_BREAKPOINT (@var{bp_tgt})
> +@itemx MEMORY_REMOVE_BREAKPOINT (@var{bp_tgt})

Same here.

Otherwise, the patch is okay with me (including moving the description
of target_insert_hw_breakpoint and target_remove_hw_breakpoint.
Thanks!