diff mbox

PR remote/21188: Fix remote serial timeout

Message ID CAL8qUbp4U+4=0CyBC+mfLAOusr+sL2NPOgqyGqo519x5GPHwpw@mail.gmail.com
State New, archived
Headers

Commit Message

Gareth McMullin March 14, 2017, 1:02 a.m. UTC 
  On Fri, Mar 3, 2017 at 5:14 AM, Simon Marchi <simon.marchi@ericsson.com> wrote:
> I think I understand the problem you describe by inspecting the code.  However,
> I have some difficulty understanding the current and proposed code, so I can't
> say if the patch looks correct.  It just looks more complicated than necessary.
>
> For example, what's the point of the timeout_remaining field in struct serial?  It
> seems to ever only be used in this function.  If we can remove it, it will be one
> less thing to consider.  We can probably have just the timeout variable that we
> decrement until it's done.

Thank you, Simon.  I only made the smallest change needed to fix the
problem.  I've attached an updated patch
to replace the timeout_remaining field with a local variable, and
remove the unused current_timeout field.

Gareth

Comments

Simon Marchi March 14, 2017, 1:40 a.m. UTC | #1 
On 2017-03-13 21:02, Gareth McMullin wrote:
> Thank you, Simon.  I only made the smallest change needed to fix the
> problem.  I've attached an updated patch
> to replace the timeout_remaining field with a local variable, and
> remove the unused current_timeout field.

Thanks for the updated patch.  I just gave it a quick look, and it looks 
good functionally.  Just a little formatting issue.  The four lines:

   else if (timeout_remaining < 0)
     continue;
   else
     return SERIAL_TIMEOUT;

should have one less indent (shift them two spaces to the left).
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 608501b..111cf4d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@ 
+2017-03-14 Gareth McMullin  <gareth@blacksphere.co.nz>
+
+	PR remote/21188
+	* ser-unix.c (do_hardwire_readchar): Wait for full timeout to elapse.
+	* serial.h (serial_t): Remove fields current_timeout and timeout_remaining.
+
 2017-03-14  Pedro Alves  <palves@redhat.com>

 	* cp-name-parser.y (cp_demangled_name_to_comp): Update comment.
diff --git a/gdb/ser-unix.c b/gdb/ser-unix.c
index b9e55f0..7f73af8 100644
--- a/gdb/ser-unix.c
+++ b/gdb/ser-unix.c
@@ -441,8 +441,6 @@  hardwire_raw (struct serial *scb)
   state.sgttyb.sg_flags &= ~(CBREAK | ECHO);
 #endif

-  scb->current_timeout = 0;
-
   if (set_tty_state (scb, &state))
     fprintf_unfiltered (gdb_stderr, "set_tty_state failed: %s\n",
 			safe_strerror (errno));
@@ -546,9 +544,21 @@  do_hardwire_readchar (struct serial *scb, int timeout)
       if (detach)
 	return SERIAL_TIMEOUT;

-      scb->timeout_remaining = (timeout < 0 ? timeout : timeout - delta);
+      int timeout_remaining = (timeout < 0 ? timeout : timeout - delta);
       status = wait_for (scb, delta);

+      if (status == SERIAL_TIMEOUT) {
+	if (timeout_remaining > 0)
+	  {
+	    timeout = timeout_remaining;
+	    continue;
+	  }
+	  else if (timeout_remaining < 0)
+	    continue;
+	  else
+	    return SERIAL_TIMEOUT;
+      }
+
       if (status < 0)
 	return status;

@@ -556,21 +566,7 @@  do_hardwire_readchar (struct serial *scb, int timeout)

       if (status <= 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
 	    return SERIAL_ERROR;	/* Got an error from read.  */
diff --git a/gdb/serial.h b/gdb/serial.h
index cf4e659..2900507 100644
--- a/gdb/serial.h
+++ b/gdb/serial.h
@@ -250,11 +250,6 @@  struct serial
 				   buffer.  -ve for sticky errors.  */
     unsigned char *bufp;	/* Current byte */
     unsigned char buf[BUFSIZ];	/* Da buffer itself */
-    int current_timeout;	/* (ser-unix.c termio{,s} only), last
-				   value of VTIME */
-    int timeout_remaining;	/* (ser-unix.c termio{,s} only), we
-				   still need to wait for this many
-				   more seconds.  */
     struct serial *next;	/* Pointer to the next `struct serial *' */
     int debug_p;		/* Trace this serial devices operation.  */
     int async_state;		/* Async internal state.  */