Re: [RFC] MI non-stop and multiprocess docs.

[Eli Zaretskii <eliz at gnu dot org>]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Tue, 4 Nov 2008 23:14:50 +0400
> 
> 
> The attached patch adds quite a pile of new MI documentation:

Thanks!

> Comments?

Some, most of them on matters of style. 

> +@node GDB/MI General Design
> +@section @sc{gdb/mi} General Design
> +
> +Interaction of a GDB/MI frontend with @value{GDBN} involves three

For consistent typesetting, please replace all literal instances of
"GDB/MI" with "@sc{gdb/mi}", like in the @section line above.

Also, I think a @cindex entry would be useful here, for those who are
seeking this kind of information.

Finally, this chapter seems to be not on design of MI, but more about
advice to frontend implementors.  So I think it should be renamed
accordingly, and some introductory text added to its beginning saying
this is the intent of the chapter.

> +parts --- commands sent to @value{GDBN}, responses to those commands

It is best not to leave spaces around "---", the results look better
in print.

> +For the commands that do not resume the target, response contains the
                                                  ^^^^^^^^^
"the response"

> +requested information.  For the commands that resume target, the
                                                       ^^^^^^^
"the target"

> +response only indicates if the target was successfully resumed.
                          ^^^^
"whether", not "if".

> +Notifications is the mechanism to report changes in the state of the
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^
"mechanism for reporting changes" is better, I think.

> +target, or in GDB state, that cannot conveniently be associated with
                 ^^^
@value{GDBN}, here and elsewhere in the patch.

> +target state -- when a target is resumed, or stopped.  It would not

"---", not "--".  Also, please remove the spaces around it.

> +different threads. Also, quite some time may pass before any event
                    ^^
Two spaces after the period that ends a sentence.  (There are more
instances of this in the patch.)

> +happens in the target, while frontend needs to know if the resuming
                               ^^^^^^^^^              ^^^^
"a frontend" and "whether".

> +diagnostic for other commands.

I think "diagnostics" (plural) is better.

>                                  Status notifications are used to

Wouldn't it be better to make "Status notifications" a separate @item
in this list?

> +There's no guarantee that whenever an MI command reports an error,
> +@value{GDBN} or the target is in any specific state, and especially,
                             ^^^^
"are"

> +processed.  Therefore, frontend is recommended, whenever an MI command
> +results in error, to refresh all information shown in the user
> +interface.

  Therefore, whenever an MI command results in an error, we recommend
  that the frontend refreshes all the information shown in the user
  interface.

(Transposing parts of the sentence makes the sentence much easier to
read and understand.)

> +done in context of specific thread and frame.  Often, even when
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"in the context of a specific thread and frame".

Also, perhaps add an @xref here to where we describe frames.

> +accessing global data, target requires that a thread be specified.  The
                         ^^^^^^^^^^^^^^^^
"the target requires" or "targets require".

> +CLI interface maintains selected thread and frame, and supplies them
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^
"the selected frame and thread"

> +command, and because user interacts with GDB with a single terminal,
                                                ^^^^^^^^^^^^^^^^^^^^^^
"via a single terminal"

> +so no confusion as to what thread and frame are current is possible.

  "so no confusion is possible as to what thread and frame are the
  current ones"

(Again, rearrangement makes the sentence more readable.)

>             Second, a graphical frontend can have more than one window,
> +and user may work with different threads in each one, and the frontend
> +might want to access other threads for internal purposes.

Style: too many "and's" in the same sentence.  I would rephrase as
follows:

   Second, a graphical frontend can have more than one window, each
   one used for debugging a different thread, and the frontend might
   want to access additional threads for internal purposes.

>                                                       Therefore, it is
> +preferrable that each MI command explicitly specify which thread and
   ^^^^^^^^^^^
"preferable".  Also, perhaps rephrase as "Therefore, we recommend that
each MI command ...".  This removes passive tense from the sentence,
which makes the sentence more concise and clear.

> +another example, if the user issues CLI @samp{thread} command via
                                      ^^
"the" is missing here.

> +frontend, it is desirable to change frontend's selected thread to the

"the frontend" and "the frontend's selected thread"

> +No such notification is available for selected frame at the moment.
                                        ^^
"the" missing.

>                         This doubles the amount of command that has
> +to be sent.

  "This doubles the number of commands that need to be sent."

"Amount" is for something like food or water that is not counted by
simple numbers.

> +if the selected thread in @value{GDBN} is supposed to be equal to the
                                                            ^^^^^
"identical" is better.

> +On some targets, @value{GDBN} is capable of processing MI commands
> +even while the target is running.  This is called asynchronous command
> +execution.

Whenever you first introduce a term, give it the @dfn markup, which
will make stand out in the text.  In this case, since this term is
explained elsewhere in the manual, it is also a good idea to add a
@pxref to that other node.

>                  When the frontend has started the executable or
> +attached to the target

I think you mean "after" rather then "when".

, it can find if asynchronous execution is
> +enabled using the @code{-list-target-features} command.
> +
> +Even if @value{GDBN} can accept a command while target is running,
> +many commands that access a target do not work when a target is
                            ^^^                       ^^^
"the" in both cases

> +When a given thread is running, MI commands that try to access the
> +target in context of that thread may not work, or may work only on
          ^^^^^^^^^^
"in the context"

>                                         Commands that read memory, or
> +modify breakpoints, may work or not work, depending on target.
                                                       ^^^^^^^^^
"on the target"

> +that even commands that operate on global state (like global
> +variables, or breakpoints), still access the target in the context of
> +a specific thread

What do you mean by "global variables" here?  As written, the text
seems to say that global variables and breakpoints are commands, or
maybe global state, which doesn't sound right to me.  "Breakpoints"
could be replaced with "breakpoint commands", but I don't know what
replacement to suggest for "global variables".

>                    so frontend should try perform such operations on a
> +stopped thread.

I don't see how this conclusion follows from the fact that commands
access the target in the context of some thread.  Why doing that "on a
stopped thread" will solve whatever problem you are trying to explain
here? and what does "operations ON a stopped thread" mean in this
context, anyway?

> +Since the set of allowed commands depends on the target, this
> +documention does not list which commands are allowed. Also,
> +@value{GDBN} does not try to simulate asynchronous execution of
> +commands that fail -- in particular, it does not try to briefly
> +interrupt an executing thread to execute a command, and it does not
> +try to automatically pick a stopped thread and execute a command in
> +the context of that thread.
> +
> +Two commands that are explicitly required to always work are
> +@code{-exec-interrupt}, to stop a thread, and @code{-thread-info}, to
> +find the state of each thread.

I fail to understand why these two paragraphs are useful, nor why they
are in this particular place.  If the first sentence of the first
paragraph is related to the second paragraph, let's put them together,
and let's also make the first sentence less general ("this
documentation", as written, seems to refer to the entire manual).

As for the second sentence of the first paragraph, I completely fail
to grasp the significance of what you are saying, or to parse the
"does not try to simulate asynchronous execution of commands that
fail" part.  Can you help me understand what you are trying to say
here?

> +In future, support for debugging several hardware systems, each with
   ^^^^^^^^^
"In the future"

> +several cores, each with different processes, is likely.

"is likely" is too far from its subject, which makes this sentence
awkward to read.  Suggest to rearrange this sentence:

  In the future, it is likely that @value{GDBN} will support debugging
  of several hardware systems, each one having several cores with
  several different processes running on each core.

Btw, talking about "the future" in a manual is problematic, because
"the future" turns out to be present at some point, and then the text
sounds obsolete for no good reason.  How about replacing "in the
future" with "on some platforms" (and getting rid of "will" and the
future tense)?

>                                                            This section
> +describes the MI mechanism to support such debugging scenarios, called
> +@emph{thread groups}.

Please use @dfn instead of @emph to introduce new terminology.  The
effect is similar, but @dfn is the markup designed specifically for
this purpose.

>                         The primary design goal of this mechanism was

I think "is" is better, again because we describe the present status
of the debugger, not its history or its future.

> +                    Every command that accepts the @samp{--thread}
> +does not care of the thread's position in the target hierarchy and

What is "the thread's position in the target hierarchy"? what
hierarchy is being referenced here?

> +To help frontend user to understand the target hierarchy,
           ^^^^^^^^^^^^^
"the frontend user" or maybe just "the user".

> +threads can be grouped in thread groups.  The 
> +@code{-list-thread-groups} command lists all the thread groups current
> +being debugged, and allows to navigate a specific thread group.

The text leaves me wondering how listing thread groups would allow "to
navigate a specific thread group".  I think this needs to be explained
or rephrased.

> +It is possible to list the thread groups available on the target
> +but not debugged yet with the @samp{-list-thread-groups --available}
> +command.

What is a thread that is not being debugged yet?  How does one "debug
a thread"?  GDB generally supports debugging of processes, and once
you debug a process, all its threads are being debugged (or so I
thought).

It sounds like you are introducing new semantics here, but if that is
so, this semantics should be explicitly and clearly explained, or else
the text would puzzle the readers.

>            What thread groups are available depends on the target, for
> +example it might be processes.

Do you mean that a thread group can be the set of all threads of a
given process?  If so, I suggest to rephrase:

  The precise meaning of a thread group depends on the target.  For
  example, the set of all the threads of a given process could define
  a thread group.

> +It is possible to attach to thread group reported by 
> +@samp{-list-thread-groups} by passing thread group id to the
> +@code{-target-attach} command.

What is a "thread group id"?  (It sounds like the answer to this is
related to the output of -list-thread-groups, but you didn't describe
that output, either.)

> +@item =thread-created,id="@var{id}",group-id="@var{gid}"
> +@itemx =thread-exited,id="@var{id}",group-id="@var{gid}"
>  A thread either was created, or has exited.  The @var{id} field
> -contains the @value{GDBN} identifier of the thread.
> +contains the @value{GDBN} identifier of the thread.  The @var{gid}
> +field identifies the thread group this thread belongs to.

Do we have the description of what exactly is a thread group id,
anywhere in the manual?  If so, please add here an xref to that
place.  If we don't have such a description, we should add one.

> +Response from many MI commands include an information about stack
                                  ^^^^^^^
Either "includes", or change "Response" to "Responses".

> +@item from
> +The name of the binary file (either executable or shared library) the
> +corresponds to the frame's code address.

For all the other @item's in this table you stated whether they are
always present or not.  What about this one?

>  Resumes the execution of the inferior program until a breakpoint is
> -encountered, or until the inferior exits.
> +encountered, or until the inferior exits.  In all-stop mode, may

A @pxref to the node that describes the all-stop mode would be useful
here.

> +resume only one thread, or all threads, depending on the value of the
> +@samp{scheduler-mode} variable.

You mean scheduler-locking, not scheduler-mode, right?  If so, please
add a reference to the node that describes it.

>                                   In non-stop mode, if the @samp{--all}
> +is not specified, only the context thread is resumed.

What is a "context thread"?

> +In non-stop mode, only the context thread is interrupted by default.

Again, I'd like a reference to where non-stop is described.

> +If an expression specified when creating a fixed variable object
> +refers to a local variable, the variable object becomes bound to the 
> +frame the local variable belongs to.

What is the meaning of a local variable _belonging_ to a frame?  Do
you mean the frame in whose scope it exists at the time of specifying
the expression for a fixed variable?

>                                       For multithreaded programs, the
> +variable object also becomes bound to the thread.

Again, what thread is that? how is it determined?

> +re-evaluates the variable object in that thread/frame.

I think you want to say "in the context of that thread/frame".

> + name="@var{name}",numchild="N",type="@var{type}",thread-id="M"

Aren't N and M numbers?  If so, please use @var{n} and @var{m}
instead.

> +With the @samp{--available option}, instead of reporting groups that are
> +been debugged, GDB will report all thread groups available on the target,
> +not only the presently debugged ones.

This sentence mentions twice the fact that not only the debugged
thread groups are reported.  One of them should be deleted.

Also, I think "option" should be outside of the braces of @samp.


> +@smallexample
> +(gdb)

This should be @value{GDBP}, I think.

> +-list-thread-groups
> +^done,threads=[],groups=[@{id="17",type="process",pid="yyy",num_children="2"@}]

The command description says it will report thread groups, but the
output shown in the example also includes "threads=[]".  What is that
part?

Thanks again for working on this.