Re: [Oleg Nesterov] PATCH? gdb remote.c: readchar() should pop_target() if SERIAL_ERROR?

[Pedro Alves <pedro@codesourcery.com>]

Oleg Nesterov wrote:

>         (gdb) q
>         A debugging session is active.
> 
>                 Inferior 1 [Remote target] will be killed.
> 
>         Quit anyway? (y or n) y
>         putpkt: write failed: Broken pipe.
>         (gdb)
> 
> Not sure my analysis is correct, but I think that readchar() needs
> a fix. A read/recv from a socket doesn't necessarily returns EOF if
> the peer has closed the socket. 

> It can return sock->sk_err set by
> the previous send if the peer dies and sets sk->sk_shutdown.

Talking in terms of Linux kernel internals, eh?
Do note that you may actually be debugging with something
not tcp, say, a bog dumb serial line, and an error may happen
at any time, not just when quitting/killing the target
(although quitting is more prone to these kinds of issues, since
the 'k' packet doesn't actually require a reply).

> The patch merely adds pop_target(). The more sophisticates fix
> should probably continue the reading, sock->sk_err was cleared and
> the socket may have the packets which we could read.

I guess the idea must have been that the user would just retry
whatever failed.  But of course, that's prone for messing
things up too.  I'm thinking this should be rare enough under normal
conditions, and that it's not worth the hassle to do anything smarter
here.  We have no clue in what state the connection or the target
was left in.  We could claim that it's just better to reconnect
and refresh the whole debug state.

> --- gdb-7.1/gdb/remote.c~       2010-03-07 15:39:53.000000000 +0100
> +++ gdb-7.1/gdb/remote.c        2010-07-02 02:38:09.000000000 +0200
> @@ -6364,6 +6364,7 @@ readchar (int timeout)
>        error (_("Remote connection closed"));
>        /* no return */
>      case SERIAL_ERROR:
> +      pop_target ();
>        perror_with_name (_("Remote communication error"));
>        /* no return */
>      case SERIAL_TIMEOUT:

... thus, I have no problems with this.  Could you tweak the
string to say something like:

 "Remote communication error.  Target disconnected."

so that user is informed we're no longer talking to the
target?.

Okay with that change.

-- 
Pedro Alves