Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Sergio Durigan Junior <sergiodj@redhat.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH 4/4 v2] Documentation and testsuite changes
Date: Fri, 06 Apr 2012 09:27:00 -0000	[thread overview]
Message-ID: <83fwchb48p.fsf@gnu.org> (raw)
In-Reply-To: <m3398hwmxh.fsf@redhat.com>

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


  reply	other threads:[~2012-04-06  9:27 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-06  3:28 [PATCH 0/4 v2] Implement support for SystemTap probes on userspace Sergio Durigan Junior
2012-04-06  3:32 ` [PATCH 1/4 v2] Refactor internal variable mechanism Sergio Durigan Junior
2012-04-06  3:36 ` [PATCH 2/4 v2] Implement new features needed for handling SystemTap probes Sergio Durigan Junior
2012-04-11 19:06   ` Jan Kratochvil
2012-04-11 22:14     ` Sergio Durigan Junior
2012-04-11 23:33       ` Jan Kratochvil
2012-04-06  3:37 ` [PATCH 4/4 v2] Documentation and testsuite changes Sergio Durigan Junior
2012-04-06  9:27   ` Eli Zaretskii [this message]
2012-04-09 21:37     ` Sergio Durigan Junior
2012-04-06  4:11 ` [PATCH 3/4 v2] Use longjmp and exception probes when available Sergio Durigan Junior
  -- strict thread matches above, loose matches on Subject: below --
2011-10-26 21:08 [PATCH] Implement new `info core mappings' command Sergio Durigan Junior
2011-10-26 21:25 ` Sergio Durigan Junior
2011-10-27  7:30   ` Eli Zaretskii
2011-10-27 18:09     ` Sergio Durigan Junior
2011-10-29 19:48       ` Eli Zaretskii
2011-10-31  0:34 ` Jan Kratochvil
2011-10-31  7:00   ` Sergio Durigan Junior
2011-10-31  8:13     ` Jan Kratochvil
2011-10-31 12:57       ` Pedro Alves
2011-11-01 11:54         ` [patch] `info proc ' completion [Re: [PATCH] Implement new `info core mappings' command] Jan Kratochvil
2011-11-01 16:23           ` Eli Zaretskii
2011-11-03 14:12             ` [patch] `info proc *' help fix [Re: [patch] `info proc ' completion] Jan Kratochvil
2011-11-03 16:57               ` Eli Zaretskii
2011-11-03 17:07                 ` Jan Kratochvil
2011-11-03 18:08                   ` Eli Zaretskii
2011-11-03 18:25                     ` Jan Kratochvil
2011-11-02 18:30           ` [patch] `info proc ' completion [Re: [PATCH] Implement new `info core mappings' command] Pedro Alves
2011-11-02 18:48             ` [commit] " Jan Kratochvil
2011-11-03 20:01       ` [PATCH] Implement new `info core mappings' command Sergio Durigan Junior
2011-11-04 10:38         ` Eli Zaretskii
2011-11-04 16:27         ` Jan Kratochvil
2011-11-08  1:49           ` Sergio Durigan Junior
2011-11-08 21:47             ` Jan Kratochvil
2011-11-09 20:32             ` Jan Kratochvil
2011-11-16  4:10               ` Sergio Durigan Junior
2011-11-21 16:15                 ` Sergio Durigan Junior
2011-11-23 16:32                   ` [rfc] Options for "info mappings" etc. (Re: [PATCH] Implement new `info core mappings' command) Ulrich Weigand
2011-11-23 23:37                     ` Sergio Durigan Junior
2011-12-01 19:51                       ` Ulrich Weigand
2011-12-05 12:59                     ` Pedro Alves
2011-12-05 15:02                       ` Ulrich Weigand
2011-12-06 16:01                         ` Pedro Alves
2011-12-06 17:19                           ` Ulrich Weigand
2011-12-07 16:29                             ` Pedro Alves
2011-12-07 17:24                               ` Pedro Alves
2011-12-07 20:14                               ` Ulrich Weigand
2011-12-09 13:28                                 ` Pedro Alves
2011-12-09 14:10                                   ` Pedro Alves
2011-12-20 23:08                                 ` Ulrich Weigand
2011-12-21 22:36                                   ` Jan Kratochvil
2011-12-22 16:15                                     ` Ulrich Weigand
2012-01-05 16:02                                   ` Pedro Alves
2012-01-05 18:03                                     ` Ulrich Weigand
2012-01-05 18:20                                       ` Pedro Alves
2012-01-05 19:54                                         ` Ulrich Weigand
2012-01-06  6:41                                           ` Joel Brobecker
2012-01-06 12:29                                             ` Pedro Alves
2012-01-06 12:27                                           ` Pedro Alves
2012-01-09 15:44                                             ` Ulrich Weigand
2012-01-11 16:38                                               ` Pedro Alves
2012-01-11 18:32                                                 ` Ulrich Weigand
2012-01-05 18:37                                       ` Mark Kettenis
2012-01-05 19:35                                         ` Ulrich Weigand
2011-04-04  3:09 [PATCH 4/6] Implement support for SystemTap probes Sergio Durigan Junior
2011-04-04 19:06 ` Eli Zaretskii
2011-04-06 20:20 ` Tom Tromey
2011-04-06 20:52   ` Sergio Durigan Junior
2011-04-07  2:41 ` Yao Qi
2011-04-07  3:32   ` Sergio Durigan Junior
2011-04-07 17:04   ` Tom Tromey
2011-04-11  3:21     ` Yao Qi
2011-04-08 12:38   ` Sergio Durigan Junior
2011-04-11  3:52     ` Yao Qi
2011-08-12 15:45     ` Jan Kratochvil
2011-08-12 17:22       ` Frank Ch. Eigler
2011-08-12 21:33         ` Sergio Durigan Junior
2011-04-19 16:42 ` Jan Kratochvil
2012-05-07 19:36   ` Jan Kratochvil
2012-05-07 19:54     ` Sergio Durigan Junior
2012-05-07 19:58       ` Jan Kratochvil
2012-05-07 20:26         ` Sergio Durigan Junior
2012-05-07 20:38           ` Jan Kratochvil
2012-05-08  1:36             ` Sergio Durigan Junior

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83fwchb48p.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=sergiodj@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox