From patchwork Thu Dec 1 19:10:09 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Michael Daniels X-Patchwork-Id: 18107 Received: (qmail 103834 invoked by alias); 1 Dec 2016 19:10:14 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 103824 invoked by uid 89); 1 Dec 2016 19:10:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-5.4 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=expired, forever, H*r:0500, H*M:blackberry X-HELO: smtp-p01.blackberry.com Received: from smtp-p01.blackberry.com (HELO smtp-p01.blackberry.com) (208.65.78.88) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 01 Dec 2016 19:10:12 +0000 Received: from xct107cnc.rim.net ([10.65.161.207]) by mhs211cnc.rim.net with ESMTP/TLS/DHE-RSA-AES256-SHA; 01 Dec 2016 14:10:10 -0500 Received: from mdaniels-linux.rim.net (10.65.160.248) by XCT107CNC.rim.net (10.65.161.207) with Microsoft SMTP Server id 14.3.319.2; Thu, 1 Dec 2016 14:10:09 -0500 Message-ID: <1480619409.3491.13.camel@blackberry.com> Subject: Re: [PATCH] Avoid premature serial timeouts From: Michael Daniels To: Luis Machado , "gdb-patches@sourceware.org" Date: Thu, 1 Dec 2016 14:10:09 -0500 In-Reply-To: <146c4bd7-ff3b-1ea5-a087-e3e6b59112f2@codesourcery.com> References: <303AEF3B642EDD4B994EA21E8DEFDC5E6F94D5@XMB126CNC.rim.net> <8a9c87a7-0c43-25a9-a8c3-9aca476de6c1@codesourcery.com> <303AEF3B642EDD4B994EA21E8DEFDC5E6FEA93@XMB126CNC.rim.net> <146c4bd7-ff3b-1ea5-a087-e3e6b59112f2@codesourcery.com> MIME-Version: 1.0 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.           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++; 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