This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH/doc] IPA protocol doc


> From: Yao Qi <yao@codesourcery.com>
> Date: Thu, 7 Jun 2012 21:33:46 +0800
> 
> This patch is to document the protocol that GDB/GDBserver is using
> with IPA, which will be used when writing a 3rd party agent.
> 

Thanks.

> gdb/doc:
> 	* gdb.texinfo (In-Process Agent Protocol): New appendix.

I wonder whether it would be better to make this a section of the
"In-Process Agent" chapter, rather than an appendix.  Any reasons not
to do that?

In any case, please mention all the new nodes in the log entry, not
just their parent.

> +                                       This chapter is to define the
> +protocol of this kind of communication.

 "This chapter documents the protocol used for communications between
 GDBserver and the IPA."

(If you move this to the chapter I suggest above, use "section" rather
than "chapter" in this sentence.)

> +in-process agent, and then in-process agent replies data back to
> +represent the return result of the command, or some extra information.

 "...and then in-process agent replies back with the return result of
 the command or some other information."

> +The data sent to in-process agent is composed by primitive data types,
                                                 ^^
"of"

> +@menu
> +* Objects:IPA Protocol Objects:
> +* Commands:IPA Protocol Commands:
> +@end menu

Please format this @menu as we do with other menus in the Texinfo
source.

> +@node IPA Protocol Objects
> +@section Objects

A @cindex entry here would be useful.  You can use the node name, just
lower-case all the words.

Also, a section name such as "Objects" is too general; why not "IPA
Protocol Objects"?

> +The commands sent to and results received from agent may contain some
> +composed type of data, which is called @dfn{object}.

 "...may contain some complex data types called @dfn{objects}."

> +                                                      Usually, object

 "Usually, an object"

> +is the unit to transfer composite data types in commands.

This sentence sounds redundant: didn't you already said precisely that
in the previous one?

> +However, there are still some differences of two ends in two processes,
                                                                         ^
A colon ':' here

> +@enumerate

Please leave an empty line before @enumerate

> +@item word size. On some 64-bit machine, @value{GDBN} or GDBserver can be
                  ^^               ^^^^^^^
Two spaces, please, and "machines" in plural.  Also, please make all
the @item's in the enumeration a separate line, like this:

  @item
  Word size.  On some 64-bit machines, ...

> +compiled in 64-bit, while in-process agent is 32-bit.
   ^^^^^^^^^^^^^^^^^^
 "compiled as a 64-bit executable"

> +@item ABI.  Some machine may have multiple types of ABI, @value{GDBN} or
                    ^^^^^^^
 "machines"

This itemized list could use a sentence or two of introductory text,
explaining what you are about to describe.

> +@itemize @bullet
> +
> +@item agent expression object

Here, too, "@item" should be on a line by itself.  (Only in a @table
or a @multitable we put text on the same line as the @item.)

> +@anchor{agent expression object}
> +@multitable @columnfractions .10 .10 .80
> +@headitem Name @tab Size @tab Description
> +@item length @tab 4 @tab length of bytes code
> +@item byte code @tab @var{length} @tab contents of byte code
> +@end multitable
> +
> +@item tracepoint action object
> +@anchor{tracepoint action object}
> +
> +@itemize @bullet
> +@item Tracepoint action for collecting memory
> +@multitable @columnfractions .10 .10 .80
> +@headitem Type @tab  Size @tab Description
> +@item 'M' @tab 1 @tab type of tracepoint action
> +@item addr @tab 8 @tab if @var{basereg} is @samp{-1}, @var{addr} is the
> +address of the lowest byte to collect, otherwise @var{addr} is the offset
> +of @var{basereg} for memory collecting.
> +@item len @tab 8 @tab length of memory for collecting
> +@item basereg @tab 4 @tab the register number containing the starting
> +memory address for collecting.
> +@end multitable

I don't think having a @multitable interspersed with an @itemize (and
nested @itemize on top of that) is a good idea.  What does it gain us?

I'd prefer if you first enumerate all the objects in a single list
(and say a sentence or two about each one, e.g. what is it used for),
and then have a single @multitable with the attributes of all of them.

> +@node IPA Protocol Commands
> +@section Commands

Again, a @cindex entry would be good here.

> +@item FastTrace:@var{tracepoint_object} @var{gdb_jump_pad_head}
> +Installs a new fast tracepoint described by @var{tracepoint_object}
> +(@pxref{tracepoint object}).  @var{gdb_jump_pad_head}, 8-byte long, is the
> +head of jumppad.

Should we tell what is a "jumppad"?

> +@item qTfSTM
> +@pxref{qTfSTM}
> +@item qTsSTM
> +@pxref{qTsSTM}
> +@item qTSTMat
> +@pxref{qTSTMat}

You want @xref here, not @pxref (the latter is only appropriate in
parentheses in the middle of a sentence).

Thanks.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]