From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29523 invoked by alias); 8 Jan 2015 15:50:42 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 29459 invoked by uid 89); 8 Jan 2015 15:50:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 08 Jan 2015 15:50:39 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t08FobSP011636 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 8 Jan 2015 10:50:37 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t08FoUsd019956; Thu, 8 Jan 2015 10:50:31 -0500 Message-ID: <54AEA745.2060703@redhat.com> Date: Thu, 08 Jan 2015 15:50:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Patrick Palka , gdb-patches@sourceware.org Subject: Re: [PATCH] Consolidate the custom TUI query hook with default query hook References: <54AE640B.3000701@redhat.com> <1420730248-18521-1-git-send-email-patrick@parcs.ath.cx> In-Reply-To: <1420730248-18521-1-git-send-email-patrick@parcs.ath.cx> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-01/txt/msg00197.txt.bz2 On 01/08/2015 03:17 PM, Patrick Palka wrote: > Hi Pedro, > > This is what I have so far. It seems to work well. Thoughts? Nice! > --- a/gdb/utils.c > +++ b/gdb/utils.c > @@ -1198,12 +1198,11 @@ compile_rx_or_error (regex_t *pattern, const char *rx, const char *message) > static int ATTRIBUTE_PRINTF (1, 0) > defaulted_query (const char *ctlstr, const char defchar, va_list args) > { > - int answer; > int ans2; > int retval; > int def_value; > char def_answer, not_def_answer; > - char *y_string, *n_string, *question; > + char *y_string, *n_string, *question, *prompt; > /* Used to add duration we waited for user to respond to > prompt_for_continue_wait_time. */ > struct timeval prompt_started, prompt_ended, prompt_delta; > @@ -1263,62 +1262,31 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args) > > /* Format the question outside of the loop, to avoid reusing args. */ > question = xstrvprintf (ctlstr, args); > + prompt = xstrprintf ("%s%s(%s or %s) %s", Add _() here, for the "or". > - printf_filtered (_("(%s or %s) "), y_string, n_string); Notice here we had it. > - > - if (annotation_level > 1) > - printf_filtered (("\n\032\032query\n")); > + char *response, answer = EOF; > > - wrap_here (""); > gdb_flush (gdb_stdout); > + response = gdb_readline_wrapper (prompt); > + if (response != NULL) > + answer = response[0]; > + xfree (response); > > - answer = fgetc (stdin); > - > - /* We expect fgetc to block until a character is read. But > - this may not be the case if the terminal was opened with > - the NONBLOCK flag. In that case, if there is nothing to > - read on stdin, fgetc returns EOF, but also sets the error > - condition flag on stdin and errno to EAGAIN. With a true > - EOF, stdin's error condition flag is not set. > - > - A situation where this behavior was observed is a pseudo > - terminal on AIX. */ > - while (answer == EOF && ferror (stdin) && errno == EAGAIN) > - { > - /* Not a real EOF. Wait a little while and try again until > - we read something. */ > - clearerr (stdin); > - gdb_usleep (10000); > - answer = fgetc (stdin); > - } > - > - clearerr (stdin); /* in case of C-d */ > if (answer == EOF) /* C-d */ > { > printf_filtered ("EOF [assumed %c]\n", def_answer); > retval = def_value; > break; > } Can you move this a bit above, and write the check in terms of response, like this? if (response == NULL) /* C-d */ { printf_filtered ("EOF [assumed %c]\n", def_answer); retval = def_value; break; } I think that's a little bit clearer. What got me to that was that you made "answer" a char (from int), which may be signed or unsigned, depending on host, and then the >= 'a' check further below looked suspicious, though I see that it's unreachable for the EOF case. I think that way there'll no be reference to EOF at all in the code (except in the printf string). Other than that, if this passes testing, that it's OK with me. Thanks for doing this. -- Pedro Alves