This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 4/4 v2] Documentation and testsuite changes
- From: Eli Zaretskii <eliz at gnu dot org>
- To: Sergio Durigan Junior <sergiodj at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Fri, 06 Apr 2012 12:26:30 +0300
- Subject: Re: [PATCH 4/4 v2] Documentation and testsuite changes
- References: <m3k41twnd4.fsf@redhat.com> <m3398hwmxh.fsf@redhat.com>
- Reply-to: Eli Zaretskii <eliz at gnu dot org>
> From: Sergio Durigan Junior <sergiodj@redhat.com>
> Date: Fri, 06 Apr 2012 00:37:14 -0300
>
> This last patch contains documentation and testsuite changes. I tried
> to address all comments that Eli has made in the first version, but I
> have to say that I could use another round of review from Eli and Tom.
Here's mine.
> +@cindex SystemTap static probe point
Although SystemTap is a proper name, I'd prefer for its index entries
to start with a lowercase letter, because otherwise an Info manual
built in some non-English locale could have the index entries sorted
in surprising ways, since the collation order of uppercase letters
differs from that in en_*.
If you think we must use "SystemTap" literally, how about moving words
around, as in
@cindex static probe point, SystemTap
> +@value{GDBN} supports @code{SDT} probes in the code. @code{SDT} stands
> +for Statically Defined Tracing, and the probes are designed to have a tiny
Since you are introducing a new term "SDT", the first time it is used
it should be in @dfn. Also, instead of @code{SDT}, I would use
@acronym{SDT} everywhere, as it really isn't a symbol in some program,
but an acronym.
> +runtime code and data footprint, and no dynamic relocations. They are
> +usable from assembly, C and C++ languages.
^^^
We use "C@t{++}" in the manual, it looks better in print.
> +@item info probes stap @r{[}@var{provider}@r{]} @r{[}@var{name}@r{]} @r{[}@var{objfile}@r{]}
I think the brackets should be nested, as you did in NEWS. I mean,
"name" can only be given if "provider" is given, and "objfile" can be
given only if the other tow are, right? So each one is not
independently optional.
> +If given, @var{provider} is a regular expression used to select which
> +providers to list. ^^^^^^^^^^^^^^^
^^^^^^^^^^^^^^^^^
"to match against provider names when selecting which probes to list".
You don't list providers, you list probes.
> If omitted, probes by all @var{providers} are listed.
Here "providers" is a literal word, so it should not have the @var
markup.
> +@item -pstap|-probe-stap @r{[}@var{objfile}:@r{]}@r{[}@var{provider}:@r{]}@var{name}
Likewise here: the brackets should be nested, I believe.
> +The @sc{gnu}/Linux tool @code{SystemTap} provides a way for
> +applications to embed static probes. @xref{Static Probe Points}, for more
> +information on finding and using static probes. This form of linespec
> +specifies the location of such a static probe.
> +
> +If @var{objfile} is given, only probes coming from that shared library
> +or executable are considered. If @var{provider} is given, then only
> +probes from that provider are considered. If several probes match the
> +spec, @value{GDBN} will insert a breakpoint at each one of those probes.
Here, you give almost no clue that these parameters can be regular
expressions. Please rewrite to make that clear, e.g.:
If @var{objfile} is given, only probes coming from a shared library
or executable matching @var{objfile} as a regular expression are
considered.
> +@cindex Semaphores on static probe points
Lower-case "semaphores", please. Index entries should all start with
a lowercase letter.
> +Some probes have an associated semaphore variable; for instance, this
> +happens automatically if you defined your probe using a DTrace-style
> +@file{.d} file. If your probe has a semaphore, @value{GDBN} will
> +automatically enable it when you specify a breakpoint using the
> +@samp{-p} notation. But, if you put a breakpoint at a probe's
> +location by some other method (e.g., @code{break file:line}), then
> +@value{GDBN} will not automatically set the semaphore.
Why is this here? Shouldn't it be where static probe points are
described?
> +@item $_probe_arg@var{n}
> +@var{n} varies from 0 to 11.
Not "varies from 0 to 11", but "is an integer between 0 and 11".
There's no variation here, as in case of a variable whose value can
change.
OK with those changes.