From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 86813 invoked by alias); 4 Sep 2017 12:35:56 -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 86271 invoked by uid 89); 4 Sep 2017 12:35:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=canceling 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 ESMTP; Mon, 04 Sep 2017 12:35:51 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C14363DBC5; Mon, 4 Sep 2017 12:35:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com C14363DBC5 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves@redhat.com Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id C8D66B930E; Mon, 4 Sep 2017 12:35:48 +0000 (UTC) Subject: Re: [PATCH 2/3] Error out immediatly when using if command without args in command list To: Simon Marchi , gdb-patches@sourceware.org References: <1504388179-579-1-git-send-email-simon.marchi@ericsson.com> <1504388179-579-3-git-send-email-simon.marchi@ericsson.com> From: Pedro Alves Message-ID: Date: Mon, 04 Sep 2017 12:35:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1504388179-579-3-git-send-email-simon.marchi@ericsson.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-09/txt/msg00038.txt.bz2 On 09/02/2017 10:36 PM, Simon Marchi wrote: > When using "if" (or while) without args directly on gdb's command line, > you get this: > > (gdb) if > if/while commands require arguments > > When doing the same when entering a command list, you only get an error > when the command is executed, when parse_exp_in_context_1 fails to > evaluate the expression. > > (gdb) define foo > Type commands for definition of "foo". > End with a line saying just "end". > >if > >end > >end > (gdb) foo > Argument required (expression to compute). > > I think it would make more sense to error out when inputting the command > list directly: > > (gdb) define foo > Type commands for definition of "foo". > End with a line saying just "end". > >if > if/while commands require arguments. > > The only required change is to check whether args is an empty string in > build_command_line. > LGTM. Tiny nit further below. BTW, as a potential improvement, we could consider also not canceling the whole command definition, but instead go back to expecting another line. It's a bit annoying to have to type everything from scratch. I've run into that occasionally with tracepoints, like: (gdb) trace foo (gdb) actions Enter actions for tracepoint 1, one per line. End with a line saying just "end". >collect ... >collect ... > #... several lines later: >endd # whoops, a typo. `endd' is not a tracepoint action, or is ambiguous. (gdb) # bah, have to start over. Instead of: (gdb) trace foo (gdb) actions Enter actions for tracepoint 1, one per line. End with a line saying just "end". >collect ... >collect ... > #... several lines later: >endd `endd' is not a tracepoint action, or is ambiguous. >end (gdb) The same safety net applied to if/while typos might be useful. Just an idea. > --- a/gdb/cli/cli-script.c > +++ b/gdb/cli/cli-script.c > @@ -147,7 +147,8 @@ build_command_line (enum command_control_type type, const char *args) > { > struct command_line *cmd; > > - if (args == NULL && (type == if_control || type == while_control)) > + if ((args == NULL || strlen (args) == 0) > + && (type == if_control || type == while_control)) > error (_("if/while commands require arguments.")); > gdb_assert (args != NULL); Nit: might not make a difference with modern compilers, though the canonical way to check for entry string would be: *args == '\0' Thanks, Pedro Alves