From 9bcbdca808b5f9fec6217d20bd4b48a56008c460 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Fri, 17 Mar 2017 16:08:12 +0000 Subject: [PATCH] PR remote/21188: Fix remote serial timeout As Gareth McMullin reports at , the timeout mechanism in ser-unix.c was broken by commit 048094acc ("target remote: Don't rely on immediate_quit (introduce quit handlers)"). Instead of applying a local fix, and since we now finally always use interrupt_select [1], let's get rid of hardwire_readchar entirely, and use ser_base_readchar instead, which has similar timeout handling, except for the bug. Smoke tested with: $ socat -d -d pty,raw,echo=0 pty,raw,echo=0 2017/03/14 14:08:13 socat[4994] N PTY is /dev/pts/14 2017/03/14 14:08:13 socat[4994] N PTY is /dev/pts/15 2017/03/14 14:08:13 socat[4994] N starting data transfer loop with FDs [3,3] and [5,5] $ gdbserver /dev/pts/14 PROG $ gdb PROG -ex "tar rem /dev/pts/15" and then a few continues/ctrl-c's, plus killing gdbserver and socat. [1] - See FIXME comments being removed. gdb/ChangeLog: 2017-03-17 Pedro Alves PR remote/21188 * ser-base.c (ser_base_wait_for): Add comment. (do_ser_base_readchar): Improve comment based on the ser-unix.c's version. * ser-unix.c (hardwire_raw): Remove reference to scb->current_timeout. (wait_for, do_hardwire_readchar, hardwire_readchar): Delete. (hardwire_ops): Install ser_base_readchar instead of hardwire_readchar. * serial.h (struct serial) : Remove fields. --- gdb/ChangeLog | 14 ++++++ gdb/ser-base.c | 14 ++++-- gdb/ser-unix.c | 152 +-------------------------------------------------------- gdb/serial.h | 5 -- 4 files changed, 25 insertions(+), 160 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 6d81cf5..ca7a49e 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,17 @@ +2017-03-17 Pedro Alves + + PR remote/21188 + * ser-base.c (ser_base_wait_for): Add comment. + (do_ser_base_readchar): Improve comment based on the ser-unix.c's + version. + * ser-unix.c (hardwire_raw): Remove reference to + scb->current_timeout. + (wait_for, do_hardwire_readchar, hardwire_readchar): Delete. + (hardwire_ops): Install ser_base_readchar instead of + hardwire_readchar. + * serial.h (struct serial) : + Remove fields. + 2017-03-17 Jonah Graham PR gdb/19637 diff --git a/gdb/ser-base.c b/gdb/ser-base.c index 3e10033..790cb1b 100644 --- a/gdb/ser-base.c +++ b/gdb/ser-base.c @@ -205,6 +205,11 @@ push_event (void *context) /* Wait for input on scb, with timeout seconds. Returns 0 on success, otherwise SERIAL_TIMEOUT or SERIAL_ERROR. */ +/* NOTE: Some of the code below is dead. The only possible values of + the TIMEOUT parameter are ONE and ZERO. OTOH, we should probably + get rid of the deprecated_ui_loop_hook call in do_ser_base_readchar + instead and support infinite time outs here. */ + static int ser_base_wait_for (struct serial *scb, int timeout) { @@ -308,10 +313,11 @@ ser_base_read_error_fd (struct serial *scb, int close_fd) } } -/* 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 -2 if timeout expired, EOF if line dropped - dead, or -3 for any other error (see errno in that case). */ +/* 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, SERIAL_EOF if line dropped dead, or SERIAL_ERROR + for any other error (see errno in that case). */ static int do_ser_base_readchar (struct serial *scb, int timeout) diff --git a/gdb/ser-unix.c b/gdb/ser-unix.c index b9e55f0..5c93891 100644 --- a/gdb/ser-unix.c +++ b/gdb/ser-unix.c @@ -78,9 +78,6 @@ struct hardwire_ttystate static int hardwire_open (struct serial *scb, const char *name); static void hardwire_raw (struct serial *scb); -static int wait_for (struct serial *scb, int timeout); -static int hardwire_readchar (struct serial *scb, int timeout); -static int do_hardwire_readchar (struct serial *scb, int timeout); static int rate_to_code (int rate); static int hardwire_setbaudrate (struct serial *scb, int rate); static int hardwire_setparity (struct serial *scb, int parity); @@ -441,155 +438,11 @@ 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)); } -/* Wait for input on scb, with timeout seconds. Returns 0 on success, - otherwise SERIAL_TIMEOUT or SERIAL_ERROR. */ - -/* FIXME: cagney/1999-09-16: Don't replace this with the equivalent - ser_base*() until the old TERMIOS/SGTTY/... timer code has been - flushed. . */ - -/* NOTE: cagney/1999-09-30: Much of the code below is dead. The only - possible values of the TIMEOUT parameter are ONE and ZERO. - Consequently all the code that tries to handle the possability of - an overflowed timer is unnecessary. */ - -static int -wait_for (struct serial *scb, int timeout) -{ - while (1) - { - struct timeval tv; - fd_set readfds; - int numfds; - - /* NOTE: Some OS's can scramble the READFDS when the select() - call fails (ex the kernel with Red Hat 5.2). Initialize all - arguments before each call. */ - - tv.tv_sec = timeout; - tv.tv_usec = 0; - - FD_ZERO (&readfds); - FD_SET (scb->fd, &readfds); - - QUIT; - - if (timeout >= 0) - numfds = interruptible_select (scb->fd + 1, &readfds, 0, 0, &tv); - else - numfds = interruptible_select (scb->fd + 1, &readfds, 0, 0, 0); - - if (numfds == -1 && errno == EINTR) - continue; - else if (numfds == -1) - return SERIAL_ERROR; - else if (numfds == 0) - return SERIAL_TIMEOUT; - - return 0; - } -} - -/* 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). */ - -/* FIXME: cagney/1999-09-16: Don't replace this with the equivalent - ser_base*() until the old TERMIOS/SGTTY/... timer code has been - flushed. */ - -/* NOTE: cagney/1999-09-16: This function is not identical to - ser_base_readchar() as part of replacing it with ser_base*() - merging will be required - this code handles the case where read() - times out due to no data while ser_base_readchar() doesn't expect - that. */ - -static int -do_hardwire_readchar (struct serial *scb, int timeout) -{ - int status, delta; - int detach = 0; - - if (timeout > 0) - timeout++; - - /* We have to be able to keep the GUI alive here, so we break the - original timeout into steps of 1 second, running the "keep the - GUI alive" hook each time through the loop. - - Also, timeout = 0 means to poll, so we just set the delta to 0, - so we will only go through the loop once. */ - - delta = (timeout == 0 ? 0 : 1); - while (1) - { - - /* 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 - quickly as possible. It is not safe to touch scb, since - someone else might have freed it. The - deprecated_ui_loop_hook signals that we should exit by - returning 1. */ - - if (deprecated_ui_loop_hook) - detach = deprecated_ui_loop_hook (0); - - if (detach) - return SERIAL_TIMEOUT; - - scb->timeout_remaining = (timeout < 0 ? timeout : timeout - delta); - status = wait_for (scb, delta); - - if (status < 0) - return status; - - status = read (scb->fd, scb->buf, BUFSIZ); - - 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) - continue; - else - return SERIAL_ERROR; /* Got an error from read. */ - } - - scb->bufcnt = status; - scb->bufcnt--; - scb->bufp = scb->buf; - return *scb->bufp++; - } -} - -static int -hardwire_readchar (struct serial *scb, int timeout) -{ - return generic_readchar (scb, timeout, do_hardwire_readchar); -} - - #ifndef B19200 #define B19200 EXTA #endif @@ -882,10 +735,7 @@ static const struct serial_ops hardwire_ops = hardwire_open, hardwire_close, NULL, - /* FIXME: Don't replace this with the equivalent ser_base*() until - the old TERMIOS/SGTTY/... timer code has been flushed. cagney - 1999-09-16. */ - hardwire_readchar, + ser_base_readchar, ser_base_write, hardwire_flush_output, hardwire_flush_input, diff --git a/gdb/serial.h b/gdb/serial.h index cf4e659..b39cc33 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. */ -- 2.7.4