This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] PR remote/21188: Fix remote serial timeout
- From: Gareth McMullin <gareth at blacksphere dot co dot nz>
- To: Simon Marchi <simon dot marchi at ericsson dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 14 Mar 2017 14:02:16 +1300
- Subject: Re: [PATCH] PR remote/21188: Fix remote serial timeout
- Authentication-results: sourceware.org; auth=none
- References: <CAL8qUbq2+mJ5OgqTmodR_g4tmQqPGYC_50W4zSBgH9N1Xnje8A@mail.gmail.com> <630aafbb-8684-5aea-0fa1-f0f538b4eb02@ericsson.com>
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
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. */