Re: [PATCH 3/3] doc, record: document record changes

[Eli Zaretskii <eliz@gnu.org>]

> From: markus.t.metzger@intel.com
> Cc: gdb-patches@sourceware.org, markus.t.metzger@gmail.com,        Markus Metzger <markus.t.metzger@intel.com>
> Date: Mon, 25 Feb 2013 17:15:17 +0100

[May I ask that you use your address fewer than 3 times in the headers?]

> Document changes to the record target resulting from the renaming into
> record-full.
> 
> Document two new record sub-commands "record instruction-history" and
> "record function-call-history" and two associated set/show commands
> "set record instruction-history-size" and "set record
> function-call-history-size".
> 
> Add this to NEWS.

Thanks.  Some comments below.

> +"record function-call-history" prints the names of the functions
> +from instructions stored in the execution log.

"prints the names of the functions called by instructions in the
execution log"

> +The process record and replay target can only debug a process that is
> +already running.  Therefore, you need first to start the process with
> +the @kbd{run} or @kbd{start} commands, and then start the recording
> +with the @kbd{record <method>} command.
> +
> +Both @code{record <method>} and @code{rec <method>} are aliases of
> +@code{target record-<method>}.

In the above, instead of "<method>", please use "@var{method}".

> +@kindex show record full memory-query
> +@item show record full memory-query

I think it is good enough to have only one "@kindex set record" and
one "@kindex show record" entry (which you already have at the
beginning of this description), without the entries that advertise the
rest of the command arguments.  These varieties are all described
together, so the multitude of index entries does not have any useful
effect, it just bloats the index.

> +                                                         Instructions
> +are printed in control-flow order.

Is that the same as the execution order?  If so, perhaps use the
latter, as I think it is more easily understandable.

> +@item record instruction-history @var{insn}, +/-@var{context}
> +Disassembles @var{context} instructions around instruction number
> +@var{insn}.  If @var{context} is preceded with @code{+}, disassembles
> +@var{context} instructions after instruction number @var{insn}.  If
> +@var{context} is preceded with @code{-}, disassembles @var{context}
> +instructions before instruction number @var{insn}.

Perhaps it would be better to use @var{n} instead of @var{context}
here.  The latter does not specifically say that it is a number.

> +@item record function-call-history
> +Print function names for instructions stored in the recorded execution
> +log.  Prints one line for each sequence of instructions that is
> +correlated to the same function.

Isn't the last sentence equivalent to saying

  Prints one line for each function call in the execution log.

?  If it is equivalent, I think my suggested wording is more clear and
less technical.

> +                                      Functions are printed in
> +control-flow order.

Again, isn't that the order of execution?

> +@item record function-call-history @var{insn}, +/-@var{context}

Same comment as above regarding @var{context}.

> +Prints @var{context} function names starting from instruction number
> +@var{insn}.  If @var{context} is preceded with @code{+}, @var{context}
> +function names are printed correlating to instructions preceding
> +@var{insn}.  If @var{context} is preceded with @code{-}, @var{context}
> +function names are printed correlating to instructions succeeding
> +@var{insn}.

Isn't this backwards?  IOW, shouldn't + print functions _after_ the
instruction, while - should print functions called _before_ the
instruction?