This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Avoid premature serial timeouts
- From: Michael Daniels <mdaniels at blackberry dot com>
- To: Luis Machado <lgustavo at codesourcery dot com>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Thu, 1 Dec 2016 14:10:09 -0500
- Subject: Re: [PATCH] Avoid premature serial timeouts
- Authentication-results: sourceware.org; auth=none
- References: <303AEF3B642EDD4B994EA21E8DEFDC5E6F94D5@XMB126CNC.rim.net> <8a9c87a7-0c43-25a9-a8c3-9aca476de6c1@codesourcery.com> <303AEF3B642EDD4B994EA21E8DEFDC5E6FEA93@XMB126CNC.rim.net> <146c4bd7-ff3b-1ea5-a087-e3e6b59112f2@codesourcery.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++;