PR remote/21188: Fix remote serial timeout
As Gareth McMullin <gareth@blacksphere.co.nz> reports at
<https://sourceware.org/ml/gdb-patches/2017-02/msg00560.html>, 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 <palves@redhat.com>
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) <current_timeout, timeout_remaining>:
Remove fields.
This commit is contained in:
parent
7503099f3e
commit
9bcbdca808
@ -1,3 +1,17 @@
|
||||
2017-03-17 Pedro Alves <palves@redhat.com>
|
||||
|
||||
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) <current_timeout, timeout_remaining>:
|
||||
Remove fields.
|
||||
|
||||
2017-03-17 Jonah Graham <jonah@kichwacoders.com>
|
||||
|
||||
PR gdb/19637
|
||||
|
@ -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)
|
||||
|
152
gdb/ser-unix.c
152
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,
|
||||
|
@ -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. */
|
||||
|
Loading…
Reference in New Issue
Block a user