RE: [patch] MI telnet service

["Abid, Hafiz" <Hafiz_Abid at mentor dot com>]

Hi Jan,
Thanks for the review. Some comments inline.

> -----Original Message-----
> From: Jan Kratochvil [mailto:jan.kratochvil@redhat.com]
> Sent: Wednesday, July 25, 2012 2:02 PM
> To: Abid, Hafiz
> Cc: gdb-patches@sourceware.org
> Subject: Re: [patch] MI telnet service
> 
> Hi Abid,
> 
> the testcase is mixing host vs. target execution.  You connect to the
> port
> from the .c program but that can run on a different machine than the
> .exp
> program is running.  It would be better to connect to the port from the
> .exp
> file, in fact you do not need any .c program there at all.  Or if the
> test
> would be done from .c file then you need to skip the test if [is_remote
> target].
I agree that it is better to just test the port from exp file and drop the c file. 

> (It could run if the target is remote but still "localhost" or "stdio"
> but
> that is currently not supported in the GDB testsuite.)
> 
> "gdbmitel" is too cryptic, the option can be very long, with words
> separated
> by dashes.
Ok.

> 
> Have you talked to Eclipse maintainers why they do not provide separate
> console?  In fact they do, I find it more fits there.  If communication
> with
> Eclipse developers has failed GDB can implement it, but I find it more
> as
> Eclipse workaround.
> 
> It does not support IPv6.  GDB currently also does not (for connections
> to
> gdbserver) but that is a bug, I tried to fix it, I hope to resurrect
> the patch
> sometimes again:
> 	[patch] IPv6 support for gdbserver
> 	http://sourceware.org/ml/gdb-patches/2006-09/msg00192.html
> 	http://sourceware.org/ml/gdb-patches/2006-10/msg00073.html
> 
> Opening the port INADDR_ANY should be possible but only with some
> additional
> -start-telnet-service option, services usually open only with
> INADDR_LOOPBACK
> by default. Still there is missing security on multi-user systems, this
> is one
> of the reasons an interface through Eclipse would be more logical to
> me.
> 
> You do not print any errors, such as in:
> +/* This is wrapper over recv. If recv returns some error then it
> closes
> +   the socket.  */
> +static int
> +receive (int *fd, void *buf, size_t n, int flags)
> I believe it could be output as:
> Node: GDB/MI Stream Records
> `"&" STRING-OUTPUT'
>      The log stream contains debugging messages being produced by GDB's
>      internals.
I will fix it.

> 
> I find it only as a last resort possibility now without readline
> support.
> Could it support $TERM negotiation like telnetd supports and attach
> readline
> to it?
Actually I have worked on negotiating the mode with telnet and then connecting the incoming data to readline. But all this was making the patch quite complicated. So I thought to go with a simple patch first and then add the readline letter. 

There has been some suggestions about making it a CLI command. But in that case, we will not be able to attach readline to the socket input as readline is already connected with CLI. I think readline cannot handle 2 parallel streams.

> 
> 
> Thanks,
> Jan