This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Save the length of inserted breakpoints
> 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!