Re: [RFA]: File-I/O patch, Documentation

["Eli Zaretskii" <eliz at is dot elta dot co dot il>]

Sorry for the delay; this is a signifcant patch, and I'm hard pressed
for free time lately.

> Date: Thu, 21 Nov 2002 10:04:43 +0100
> From: Corinna Vinschen <vinschen@redhat.com>
> 
> this is the patch containing the additional documentation for File-I/O.

Thanks.  Your documentation patch is approved, provided that you
correct the following technicalities before committing it:

> +@menu
> +* File-I/O Overview::
> +* Protocol basics::
> +* The @code{F} request packet::
> +* The @code{F} reply packet::
> +* Memory transfer::
> +* The Ctrl-C message::
> +* Console I/O::
> +* The isatty(3) call::
> +* The system(3) call::
> +* List of supported calls::
> +* Protocol specific representation of datatypes::
> +* Constants::
> +@end menu

There are a few problems here.  This menu lists node names, some of
which use characters and constructs that are either disallowed or
discouraged in node names.  You cannot use parentheses in node names,
since (foo)bar as a node name means node `bar' in the manual `foo'.
Commands like @code are discouraged in node names, because using them
makes validation of cross-references hard.

See the node "Node Line Requirements" in the Texinfo manual for more
details.

The corresponding @node lines should be fixed as well.

The corresponding section and subsection names, by contrast, may use
those characters.  In fact, you are encouraged to use markup such as
@code in section names, as that makes the printed manual (where only
the section names are visible) look prettier.

> +This simulates file system operations even on file system-less targets. 

A stylistic nit: I'd suggest to say "targets that lack file
systems".  I think it sounds less awkward.

> +@smallexample
> +(gdb) continue
   ^^^^^
This should be @value{GDBP}...

> +  target is stopped, GDB executes system call
                        ^^^
...and this should be @value{GDBN}.

> +@item
> +All parameters to the system call.  Pointers are given as addresses
> +into the target memory.

I suggest to say "as addresses in the target memory address space".

> +@item
> +Translating all values from protocol representation to host representation
> +as needed.

I think it's better to say "@value{GDBN} translates all values...",
to be consistent with the wording of the previous @item.  And the
same below:

> +@item
> +Call system call.

"@value{GDBN} calls the system call."

> +@item
> +Coerce datatypes back to protocol representation.

"It then coerces datatypes back..."

> +@item
> +Errno, if has been changed by the system call.

I think @code{errno} is better.

> +@item
> +"Ctrl-C" flag.

Please use `` and '' instead of " and " in Texinfo.  They produce
identical results in the Info output, but the former are typeset
better by TeX in the printed manual.

> +@item @code{F}@var{retcode}@code{,}@var{errno}@code{,}@var{Ctrl-C flag}@code{;}@var{call specific attachment}

This is okay, but you don't need to repeat @code all that much, just
include everything in a @code to begin with.  I think the following
will produce the same effect as what you did:

 @item @code{F@var{retcode},@var{errno},@var{Ctrl-C flag};@var{call-specific attachment}}

> +@smallexample
> +F0,0,C
> +@end smallexample
> +
> +or, if the call was interupted before the host call has been performed:

There should be a @noindent (on a line by itself) before the last
line, to prevent makeinfo and TeX from indenting it.

> +@smallexample
> +F-1,4,C
> +@end smallexample
> +
> +assuming 4 is the protocol specific representation of EINTR.

Same here.

> +Structured data which is transferred using a memory read or write as e. g.

Please use "e.g.@:" instead of "e. g.".  The @: tells TeX that the
period does not end a sentence, so that TeX won't typeset extra white
space after it.

> +the target before the @code{F} packet is sent resp. by @value{GDBN} before

Same here: please use "resp.@:".

> +gotten a break message.  The meaning for the target is "system call
> +interupted by SIGINT".

Please use `` and ''.

> +@itemize @bullet
> +@item
> +The system call hasn't been performed on the host yet.
> +
> +@item The system call on the host has been finished.

Please leave the @item on its separate line in the last sentence.

> +These two states can be distinguished by the target by the value of the
> +returned errno.  If it's the protocol representation of EINTR, the system

"errno" should be in @code, and so should be "EINTR" and any other
possible values of `errno'.

> +on POSIX systems.  In any other case, the target may presume that the
> +system call has been finished -- successful or not -- and should behave
> +as if the break message arrived right after the system call.

Whenever you want a long dash, please write "---" (3 dashes) in
Texinfo.  "--" and "-" produce identical results in the printed
manual, while "---" produces a longer dash.

> +@value{GDBN} must behave reliable.  If the system call has not been called
> +yet, @value{GDBN} may send the 'F' reply immediately, setting EINTR as

The 'F' should be @code{F}, I think.

> +(@code{write(1, ...)} or @code{write(2, ...)}).  Console input is handled

Please use @dots{} instead of literal "...".

> +The user presses Ctrl-C.

What the user types should have the @kbd markup: "@kbd{Ctrl-C}".

>                            The behaviour is as explained above, the read()
> +system call is treated as finished.

Please say "@code{read}" instead of "read()".  The latter looks like
a call to the function `read' with no arguments, which is not what
you want to say.

In general, the frequent practice of appending "()" to a name to mean
that it's a function should be abandoned in Texinfo in favor of the
@code markup.

> +The user presses <Enter>.

Keys should have the @key markup: "@key{Enter}".  This produces a
small image of a key in the printed manual.

> +Due to security concerns, the system(3) call is refused to be called
> +by @value{GDBN}.

I think you forgot to say "by default" here.  GDB only refises to call
`system' by default, not unconditionally.

>                     The user has to allow this call explicitely by 
                                                      ^^^^^^^^^^^
This should be "explicitly".

> +@table @samp
> +@item @code{set remote system-call-allowed 1}
> +@end table
> +
> +Disabling the system(3) call is done by
> +
> +@table @samp
> +@item @code{set remote system-call-allowed 0}
> +@end table
> +
> +The current setting is shown by typing
> +
> +@table @samp
> +@item @code{show remote system-call-allowed}
> +@end table

The "set remote system-call-allowed" and "show remote
system-call-allowed" commands should be indexed with @kindex, like we
do with all user commands.

> +The integral datatypes used in the system calls are
> +
> +@smallexample
> +int, unsigned int, long, unsigned long, mode_t and time_t
> +@end smallexample

I'd put the "and" and the commas inside @r{}, so that they don't have
the fixed typewriter typeface used inside @smallexample.

> +Int, unsigned int, mode_t and time_t are implemented as 32 bit values
> +in this protocol.
> +
> +Long and unsigned long are implemented as 64 bit types. 

The names of the data types (int, mode_t, etc.) should have the @code
markup here.

> +@xref{Limits}, for corresponding MIN and MAX values (similar to those
> +in limits.h) to allow range checking on host and target.

"limits.h" should have the @file markup, as it's a file name.

> +Pointers to target data is transmitted as they are.
                           ^^
This should be "are".

>                                                       A difference
> +is made for pointers to buffers for which the length isn't

"An exception is made for pointers...".

> +transmitted as part of the function call, namely strings.  Strings
> +are transmitted as a pointer/length pair, both as hex values, e. g.

"e.g.@:"

> +@smallexample
> +@code{1aaf/12}
> +@end smallexample
> +
> +which is a pointer to data of length 18 bytes at position 0x1aaf.

Please insert a @noindent before the last line.

> +@smallexample
> +"hello, world" at address 0x123456
> +@end smallexample
> +
> +is transmitted as

Same here.

> +All values are given in hexadecimal representation.
> +
> +@smallexample
> +  O_RDONLY        0
> +  O_WRONLY        1
> +  O_RDWR          2
> +  O_APPEND        8
> +  O_CREAT       200
> +  O_TRUNC       400
> +  O_EXCL        800
> +@end smallexample
> +
> +@node mode_t values
> +@unnumberedsubsubsec mode_t values
> +
> +All values are given in octal representation.

Is it really necessary to use hexadecimal representation for the
former and octal representation for the latter?  Can we use the same
representation for both, to avoid confusion?

Finally, please add @cindex entries to allow for efficient searches
of relevant issues.  As a minimum, please consider adding a @cindex
for section/subsection whose name is the same as the section name,
but in lower case.  For example:

  @node Memory transfer
  @subsection Memory transfer
  @cindex memory transfer, in file I/O packets

(The addition of "in file I/O packets" is meant to qualify "memory
transfer" which is too general; without it, a user might not realize
this index entry is for the file I/O feature.)

Thanks again for writing this.