Re: [PATCH 4/4 v2] Documentation and testsuite changes

Eli Zaretskii
2012-04-06T09:27:24Z

> 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.