From patchwork Tue Mar 14 01:02:16 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gareth McMullin X-Patchwork-Id: 19555 Received: (qmail 32289 invoked by alias); 14 Mar 2017 01:02:45 -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 32216 invoked by uid 89); 14 Mar 2017 01:02:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.2 spammy=5469, H*F:D*nz, difficulty, H*F:D*co.nz X-HELO: mail-vk0-f46.google.com Received: from mail-vk0-f46.google.com (HELO mail-vk0-f46.google.com) (209.85.213.46) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 14 Mar 2017 01:02:38 +0000 Received: by mail-vk0-f46.google.com with SMTP id x75so44598145vke.2 for ; Mon, 13 Mar 2017 18:02:38 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=IaMweh3BmtfOHKb0IhUdNQplEPL2ZpD5dYnQDnWOoyQ=; b=OASTTJemCcdC++OVXshglssgj+KDGot8a4YqvuNIAGvFLx5M3duO0prBDpmJTX5Trc np5Uo2ovw45P/4iVgA3T8BuWIwqp9eXnGrbv23qWxznkxP6BYsHyiRiszqgkYFkn5dEM J6zg4NJSttV6KjuLw/8cGI/RHUd2kwJcBJ+cQYyopGbGANmL7NdeQwwmXti2KPxqxZN4 HAAuoyl4udZNd5WOOypm21Hflh01BD8cvY6T71AjQA4nnCLw5WFHv6E4ShpeOENqWAMr KIWU+Xfc5aXHVGFKcwIUN1QiMjJtLOnPC2eYjUZclsZHOSio/7hz+LiV5cbCFtOuFjbS Lq1A== X-Gm-Message-State: AMke39nySnrpFbb6XPd39dKAz4wkbf6DKwLGKa4/FtRZWQnTCIPobCztqqcvZ0bAeqP23V/Co+hcmwy1gd0APw== X-Received: by 10.31.93.66 with SMTP id r63mr16531042vkb.126.1489453357301; Mon, 13 Mar 2017 18:02:37 -0700 (PDT) MIME-Version: 1.0 Received: by 10.31.102.68 with HTTP; Mon, 13 Mar 2017 18:02:16 -0700 (PDT) In-Reply-To: <630aafbb-8684-5aea-0fa1-f0f538b4eb02@ericsson.com> References: <630aafbb-8684-5aea-0fa1-f0f538b4eb02@ericsson.com> From: Gareth McMullin Date: Tue, 14 Mar 2017 14:02:16 +1300 Message-ID: Subject: Re: [PATCH] PR remote/21188: Fix remote serial timeout To: Simon Marchi Cc: gdb-patches@sourceware.org X-IsSubscribed: yes On Fri, Mar 3, 2017 at 5:14 AM, Simon Marchi 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 + + 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 * 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. */