Re: [PATCH] Avoid premature serial timeouts

[Michael Daniels <mdaniels at blackberry dot com>]

On Wed, 2016-11-23 at 11:57 -0600, Luis Machado wrote:

> Thanks. The new patch looks good to me. You'll need a ChangeLog entry

Will do, and thanks for the help.

> Did you exercise this patch against gdb/gdbserver with the testsuite?

I ran the test suite with the QNX 'pdebug' protocol/target over serial.
Without the patch the tests constantly hit the problem and after the
patch they run as well as they do over tcp, just much slower.

> make check-gdb RUNTESTFLAGS="--target_board native-gdbserver"

That will test over tcp, so it won't end up hitting the changed code. I
did try to hack up the test files to try and get it to use gdbserver
over a pty, but I could not seem to get it to run reliably regardless
of whether the change was applied or not.

> On a side note, your mail agent seems to be inserting strange
> characters for spaces and '='.

Hmm. Trying a new client that seems to handle this better. Will repost
the patch below.


gdb/ChangeLog:

* ser-unix.c: (do_hardwire_readchar): Rearrange code so it won't
prematurely return a timeout after the first 1 second wait_for.

diff --git a/gdb/ser-unix.c b/gdb/ser-unix.c
index 2e7b1b4..68ed24e 100644
--- a/gdb/ser-unix.c
+++ b/gdb/ser-unix.c
@@ -500,8 +500,8 @@ wait_for (struct serial *scb, int timeout)
 /* Read a character with user-specified timeout.  TIMEOUT is number of
    seconds to wait, or -1 to wait forever.  Use timeout of 0 to effect
    a poll.  Returns char if successful.  Returns SERIAL_TIMEOUT if
-   timeout expired, EOF if line dropped dead, or SERIAL_ERROR for any
-   other error (see errno in that case).  */
+   timeout expired, SERIAL_EOF if line dropped dead, or SERIAL_ERROR
+   for any other error (see errno in that case).  */
 
 /* FIXME: cagney/1999-09-16: Don't replace this with the equivalent
    ser_base*() until the old TERMIOS/SGTTY/... timer code has been
@@ -516,7 +516,7 @@ wait_for (struct serial *scb, int timeout)
 static int
 do_hardwire_readchar (struct serial *scb, int timeout)
 {
-  int status, delta;
+  int delta;
   int detach = 0;
 
   if (timeout > 0)
@@ -532,6 +532,8 @@ do_hardwire_readchar (struct serial *scb, int
timeout)
   delta = (timeout == 0 ? 0 : 1);
   while (1)
     {
+      int status;
+      ssize_t byte_count;
 
       /* N.B. The UI may destroy our world (for instance by calling
          remote_stop,) in which case we want to get out of here as
@@ -549,34 +551,27 @@ do_hardwire_readchar (struct serial *scb, int
timeout)
       scb->timeout_remaining = (timeout < 0 ? timeout : timeout -
delta);
       status = wait_for (scb, delta);
 
-      if (status < 0)
+      if (status == SERIAL_TIMEOUT && scb->timeout_remaining != 0)
+	{
+	  timeout = scb->timeout_remaining;
+	  continue;
+	}
+      else if (status < 0)
 	return status;
 
-      status = read (scb->fd, scb->buf, BUFSIZ);
+      byte_count = read (scb->fd, scb->buf, BUFSIZ);
 
-      if (status <= 0)
+      if (byte_count == 0)
+	return SERIAL_EOF;
+      else if (byte_count < 0)
 	{
-	  if (status == 0)
-	    {
-	      /* Zero characters means timeout (it could also be EOF,
but
-	         we don't (yet at least) distinguish).  */
-	      if (scb->timeout_remaining > 0)
-		{
-		  timeout = scb->timeout_remaining;
-		  continue;
-		}
-	      else if (scb->timeout_remaining < 0)
-		continue;
-	      else
-		return SERIAL_TIMEOUT;
-	    }
-	  else if (errno == EINTR)
+	  if (errno == EINTR)
 	    continue;
-	  else
+          else
 	    return SERIAL_ERROR;	/* Got an error from read.  */
 	}
 
-      scb->bufcnt = status;
+      scb->bufcnt = byte_count;
       scb->bufcnt--;
       scb->bufp = scb->buf;
       return *scb->bufp++;