[3/4] GDB: Fix documentation for invoking GDBSERVER

  The documentation did not mention the possibility of invoking gdbserver
with the new connection forms such as tcp6:host:port.  This change fixes
that.

gdb/doc/

  * gdb.texinfo (Server): Tabulate the various permitted forms of the @var{comm}
   metasyntactical variable.  Include the unix:@var{host}:@var{socket} form as
   one of them.
---
 gdb/doc/gdb.texinfo | 60 +++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 44 insertions(+), 16 deletions(-)
  

> From: John Darrington <john@darrington.wattle.id.au>
> Cc: John Darrington <john@darrington.wattle.id.au>
> Date: Sat, 13 Oct 2018 19:58:00 +0200
> 
> The documentation did not mention the possibility of invoking gdbserver
> with the new connection forms such as tcp6:host:port.  This change fixes
> that.

Thanks.  Please see a few comments below.

> +@table @code
> +@item @code{@var{device}}
> +A serial line device.

Since your @table uses @code by default, there's no need to use @code
in each @item of the table.

> +@var{host} must either be the null string or the literal string @code{localhost}.

What is a "null string"?  Do you mean an empty string?

Thanks.
  

On Saturday, October 13 2018, John Darrington wrote:

> The documentation did not mention the possibility of invoking gdbserver
> with the new connection forms such as tcp6:host:port.  This change fixes
> that.

Thanks for the patch.

As I mentioned while reviewing patch #1, there is a reason why gdbserver
doesn't accept prefixes (which means that there isn't a mistake in the
docs).  If you go with my suggestion and implement just the support for
the "unix:" prefix, then this patch will need to be changed because it
still won't be correct to mention the "tcp" prefixes.

> gdb/doc/
>
>   * gdb.texinfo (Server): Tabulate the various permitted forms of the @var{comm}
>    metasyntactical variable.  Include the unix:@var{host}:@var{socket} form as
>    one of them.
> ---
>  gdb/doc/gdb.texinfo | 60 +++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 44 insertions(+), 16 deletions(-)
>
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 1e97d692b6..e3b62221cf 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -21097,9 +21097,19 @@ syntax is:
>  target> gdbserver @var{comm} @var{program} [ @var{args} @dots{} ]
>  @end smallexample
>  
> -@var{comm} is either a device name (to use a serial line), or a TCP
> -hostname and portnumber, or @code{-} or @code{stdio} to use
> -stdin/stdout of @code{gdbserver}.
> +@code{gdbserver} waits passively for the host @value{GDBN} to communicate
> +with it.
> +
> +@var{comm} may take several forms:
> +
> +@table @code
> +@item @code{@var{device}}
> +A serial line device.
> +
> +@item @code{-}
> +@itemx @code{stdio}
> +To use the stdin/stdout of @code{gdbserver}.
> +
>  For example, to debug Emacs with the argument
>  @samp{foo.txt} and communicate with @value{GDBN} over the serial port
>  @file{/dev/com1}:
> @@ -21108,8 +21118,27 @@ For example, to debug Emacs with the argument
>  target> gdbserver /dev/com1 emacs foo.txt
>  @end smallexample
>  
> -@code{gdbserver} waits passively for the host @value{GDBN} to communicate
> -with it.
> +The @code{stdio} connection is useful when starting @code{gdbserver}
> +with ssh:
> +
> +@smallexample
> +(gdb) target remote | ssh -T hostname gdbserver - hello
> +@end smallexample
> +
> +The @samp{-T} option to ssh is provided because we don't need a remote pty,
> +and we don't want escape-character handling.  Ssh does this by default when
> +a command is provided, the flag is provided to make it explicit.
> +You could elide it if you want to.
> +
> +Programs started with stdio-connected gdbserver have @file{/dev/null} for
> +@code{stdin}, and @code{stdout},@code{stderr} are sent back to gdb for
> +display through a pipe connected to gdbserver.
> +Both @code{stdout} and @code{stderr} use the same pipe.
> +
> +@item @code{@var{host}:@var{port}}
> +@itemx @code{tcp:@var{host}:@var{port}}
> +@itemx @code{tcp4:@var{host}:@var{port}}
> +To use a @acronym{TCP} @acronym{IPv4} socket connection on port number @var{port}.
>  
>  To use a TCP connection instead of a serial line:
>  
> @@ -21129,22 +21158,21 @@ conflicts with another service, @code{gdbserver} prints an error message
>  and exits.}  You must use the same port number with the host @value{GDBN}
>  @code{target remote} command.
>  
> -The @code{stdio} connection is useful when starting @code{gdbserver}
> -with ssh:
> +
> +@item @code{tcp6:@var{host}:@var{port}}
> +To use a @acronym{TCP} @acronym{IPv6} socket connection on port number @var{port}.
> +
> +@item @code{unix:@var{host}:@var{local-socket}}
> +To use a Unix domain socket.  This will create a socket with the file
> +system entry @var{local-socket} and listen on that.  For example:
>  
>  @smallexample
> -(gdb) target remote | ssh -T hostname gdbserver - hello
> +target> gdbserver unix:localhost:/tmp/gdb-socket0 emacs foo.txt
>  @end smallexample
>  
> -The @samp{-T} option to ssh is provided because we don't need a remote pty,
> -and we don't want escape-character handling.  Ssh does this by default when
> -a command is provided, the flag is provided to make it explicit.
> -You could elide it if you want to.
> +@var{host} must either be the null string or the literal string @code{localhost}.
> +@end table
>  
> -Programs started with stdio-connected gdbserver have @file{/dev/null} for
> -@code{stdin}, and @code{stdout},@code{stderr} are sent back to gdb for
> -display through a pipe connected to gdbserver.
> -Both @code{stdout} and @code{stderr} use the same pipe.
>  
>  @anchor{Attaching to a program}
>  @subsubsection Attaching to a Running Program
> -- 
> 2.11.0
  

On Thu, Oct 18, 2018 at 04:27:24PM -0400, Sergio Durigan Junior wrote:
     On Saturday, October 13 2018, John Darrington wrote:
     
     > The documentation did not mention the possibility of invoking gdbserver
     > with the new connection forms such as tcp6:host:port.  This change fixes
     > that.
     
     Thanks for the patch.
     
     As I mentioned while reviewing patch #1, there is a reason why gdbserver
     doesn't accept prefixes (which means that there isn't a mistake in the
     docs).  If you go with my suggestion and implement just the support for
     the "unix:" prefix, then this patch will need to be changed because it
     still won't be correct to mention the "tcp" prefixes.
     
Thanks for the reviews.

How about instead, we just add a test to print an error/warning if
someone tries to start gdbserver with a udp prefix?

J'
  

On Friday, October 19 2018, John Darrington wrote:

> On Thu, Oct 18, 2018 at 04:27:24PM -0400, Sergio Durigan Junior wrote:
>      On Saturday, October 13 2018, John Darrington wrote:
>      
>      > The documentation did not mention the possibility of invoking gdbserver
>      > with the new connection forms such as tcp6:host:port.  This change fixes
>      > that.
>      
>      Thanks for the patch.
>      
>      As I mentioned while reviewing patch #1, there is a reason why gdbserver
>      doesn't accept prefixes (which means that there isn't a mistake in the
>      docs).  If you go with my suggestion and implement just the support for
>      the "unix:" prefix, then this patch will need to be changed because it
>      still won't be correct to mention the "tcp" prefixes.
>      
> Thanks for the reviews.
>
> How about instead, we just add a test to print an error/warning if
> someone tries to start gdbserver with a udp prefix?

Hm, I thought about that while writing the email, but at the time it
didn't seem like a good option.  However, thinking about it again, I
think it would be mostly fine.  I guess it's OK to support the
"tcp{,6}:" prefixes.

The check needs to implemented on gdbserver/remote-utils.c:remote_open,
and gdbserver needs to error out if "hint.ai_protocol == IPPROTO_UDP".
And "parse_connection_spec_without_prefix" could be merged with
"parse_connection_spec".

Aside from the documentation, you'd also probably have to update the
tests (both the selftests and the regular ones).

Thanks,
  

On Fri, Oct 19, 2018 at 04:45:14PM -0400, Sergio Durigan Junior wrote:

     The check needs to implemented on gdbserver/remote-utils.c:remote_open,
     and gdbserver needs to error out if "hint.ai_protocol == IPPROTO_UDP".
     And "parse_connection_spec_without_prefix" could be merged with
     "parse_connection_spec".
     
A quick test shows that this is not necessary.  Any attempt to start
gdbserver with udp: will never get to remote_open because remote_prepare
will fail when it gets to listen:
 
  "Can't listen on socket: Operation no supported."

(listen is for connection orientated protocols only).


J'
  

On Sunday, October 21 2018, John Darrington wrote:

> On Fri, Oct 19, 2018 at 04:45:14PM -0400, Sergio Durigan Junior wrote:
>
>      The check needs to implemented on gdbserver/remote-utils.c:remote_open,
>      and gdbserver needs to error out if "hint.ai_protocol == IPPROTO_UDP".
>      And "parse_connection_spec_without_prefix" could be merged with
>      "parse_connection_spec".
>      
> A quick test shows that this is not necessary.  Any attempt to start
> gdbserver with udp: will never get to remote_open because remote_prepare
> will fail when it gets to listen:
>  
>   "Can't listen on socket: Operation no supported."
>
> (listen is for connection orientated protocols only).

Yes, but this error message is not entirely clear.  We should show a
more user-friendly message, and the best way to do that is to do an
early check for the UDP protocol.

Thanks,
  

On Fri, Oct 19, 2018 at 04:45:14PM -0400, Sergio Durigan Junior wrote:

     Aside from the documentation, you'd also probably have to update the
     tests (both the selftests and the regular ones).
     

When I run "make check" I get  a large number of failures, and I'm told
that this is normal.   How do I run just the tests which you think need
to be adjusted?

J'
  

On Tuesday, October 23 2018, John Darrington wrote:

> On Fri, Oct 19, 2018 at 04:45:14PM -0400, Sergio Durigan Junior wrote:
>
>      Aside from the documentation, you'd also probably have to update the
>      tests (both the selftests and the regular ones).
>      
>
> When I run "make check" I get  a large number of failures, and I'm told
> that this is normal.   How do I run just the tests which you think need
> to be adjusted?

You can take a look at the tests I touched when implementing IPv6:

commit c7ab0aef11d91b637bf091aa9176b8dc4aadee46
Author: Sergio Durigan Junior <sergiodj@redhat.com>
Date:   Fri May 18 01:29:24 2018 -0400

    Implement IPv6 support for GDB/gdbserver


Specifically gdb.server/server-connect.exp.  Offhand I'm not sure if any
other test needs to be adjusted; probably not.  I'd advise taking a look
under the gdb.server/ directory just to double check.

Thanks,