Re: [PATCH] Conditional tracepoints

[Eli Zaretskii <eliz@gnu.org>]

> Date: Fri, 26 Jun 2009 22:31:07 -0700
> From: Stan Shebs <stan@codesourcery.com>
> 
> This patch adds conditional tracepoints.

Thanks.

I think we need a NEWS entry for this.

> ==> doc/ChangeLog <==
> 2009-06-26  Stan Shebs  <stan@codesourcery.com>
> 
>     * gdb.texinfo (Tracepoint Conditions): New section.
>     (General Query Packets): Describe ConditionalTracepoints.
>     (Tracepoint Packets): Describe condition field.
>     (Maintenance Commands): Describe maint agent-eval.

This part is okay, with the following comments:

> + programming language (@pxref{Expressions, ,Expressions}).  A
> + tracepoint with a condition evaluates the expression each time your
> + program reaches it, and data collection happens only if the condition
> + is @emph{true}.

Not sure why you use the @emph markup here.  I think the text is okay
without any markup at all.

> + In contrast to breakpoint conditions, @value{GDBN} does not actually
> + evaluate @var{expression} at the time the @code{condition} command (or
> + a command that sets a tracepoint with a condition, like @code{trace if
> + @dots{}}) is given, however.  @xref{Expressions, ,Expressions}.

Two comments:

  . The @var{expression} does not reference anything in this section,
    so the reader might become confused about what expression you had
    in mind.
  . You seem to imply that the condition is evaluated by conditional
    breakpoints when the command is given.  If so, that's not true,
    AFAIK: the condition is evaluated each time a breakpoint is hit.
    So the "in contrast to ..." part is not really right here.

> + Instead, @value{GDBN} encodes the expression into a form suitable for
> + execution on the target, independently of @value{GDBN}. Global
                                                          ^^
Only one space.

> + @smallexample
> + (@value{GDBP}) @b{trace normal_operation if errcode > 0}
> + @end smallexample

Why the @b markup here?  We don't use that elsewhere in examples.

>   @table @code
>   @kindex maint agent
> + @kindex maint agent-eval
>   @item maint agent @var{expression}
> + @itemx maint agent-eval @var{expression}
>   Translate the given @var{expression} into remote agent bytecodes.
>   This command is useful for debugging the Agent Expression mechanism
> ! (@pxref{Agent Expressions}).  The @samp{agent} version produces an
> ! expression useful for data collection, such as by tracepoints, while
> ! @samp{maint agent-eval} produces an expression that evaluates directly
> ! to a result.

I must admit that I don't understand the description you added.  What
does "expression useful for data collection" mean, and how is it
different from "expression that evaluates directly to a result"?

> + @item ConditionalTracepoints
> + The remote stub accepts and tests conditional expressions defined for
> + tracepoints (@pxref{Tracepoint Conditions}).

Don't you mean "accepts and supports" or "accepts and implements"?