From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9611 invoked by alias); 29 Jun 2017 22:21:26 -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 9579 invoked by uid 89); 29 Jun 2017 22:21:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= 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; Thu, 29 Jun 2017 22:21:24 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E902B85541; Thu, 29 Jun 2017 22:21:22 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E902B85541 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=sergiodj@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E902B85541 Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A104C77EED; Thu, 29 Jun 2017 22:21:22 +0000 (UTC) From: Sergio Durigan Junior To: Simon Marchi Cc: GDB Patches , Jerome Guitton Subject: Re: [PATCH] PR cli/21688: Fix multi-line/inline command differentiation References: <20170629020527.468-1-sergiodj@redhat.com> <90d0a1563dea6893b5dbcd8df19d0285@polymtl.ca> <87d19mmv7q.fsf@redhat.com> Date: Thu, 29 Jun 2017 22:21:00 -0000 In-Reply-To: (Simon Marchi's message of "Thu, 29 Jun 2017 23:06:21 +0200") Message-ID: <87fueil9jx.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg00802.txt.bz2 On Thursday, June 29 2017, Simon Marchi wrote: > On 2017-06-29 21:48, Sergio Durigan Junior wrote: >> On Thursday, June 29 2017, Simon Marchi wrote: >>> Another (maybe simpler) way would be to check >>> >>> else if (command_name_equals (cmd, "python") && *cmd_name == '\0') >>> >>> It's not clear when expressed like this though because cmd_name is not >>> well named at this point (it points just after the command name). >> >> Hm, right. Would you prefer this way instead? I don't have a strong >> opinion on this. > > My opinion is the solution with the least code is probably best, if > they are equivalent otherwise, but I don't really mind. It's just a > suggestion. Right. I did some more tests here, and unfortunately your solution doesn't work for all cases. For example, if the user puts trailing whitespace on the command name (like "python "), *cmd_name will point to a whitespace after the call to lookup_cmd_1. So here's second version of the patch, with the fixes you requested except the one above. WDYT? -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ >From 5c80c97c4a11ec954cc5d426e77cf209dc43f10d Mon Sep 17 00:00:00 2001 From: Sergio Durigan Junior Date: Wed, 28 Jun 2017 21:55:03 -0400 Subject: [PATCH] PR cli/21688: Fix multi-line/inline command differentiation This bug is a regression caused by the following commit: 604c4576fdcfc4e7c28f569b3748a1b6b4e0dbd4 is the first bad commit commit 604c4576fdcfc4e7c28f569b3748a1b6b4e0dbd4 Author: Jerome Guitton Date: Tue Jan 10 15:15:53 2017 +0100 The problem happens because, on cli/cli-script.c:process_next_line, GDB is not using the command line string to identify which command to run, but it instead using the 'struct cmd_list_element *' that is obtained by using the mentioned string. The problem with that is that the 'struct cmd_list_element *' doesn't have any information on whether the command issued by the user is a multi-line or inline one. A multi-line command is a command that will necessarily be composed of more than 1 line. For example: (gdb) if 1 >python >print ('hello') >end >end As can be seen in the example above, the 'python' command actually "opens" a new command line (represented by the change in the indentation) that will then be used to enter Python code. OTOH, an inline command is a command that is "self-contained" in a single line, for example: (gdb) if 1 >python print ('hello') >end This Python command is a one-liner, and therefore there is no other Python code that can be entered for this same block. There is also no change in the indentation. So, the fix is somewhat simple: we have to revert the change and use the full command line string passed to process_next_line in order to identify whether we're dealing with a multi-line or an inline command. This commit does just that. As can be seen, this regression also affects other languages, like guile or the compile framework. To make things clearer, I decided to create a new helper function responsible for identifying a non-inline command. Testcase is attached. gdb/ChangeLog: 2017-06-29 Sergio Durigan Junior PR cli/21688 * cli/cli-script.c (command_name_equals_not_inline): New function. (process_next_line): Adjust 'if' clauses for "python", "compile" and "guile" to use command_name_equals_not_inline. gdb/testsuite/ChangeLog: 2017-06-29 Sergio Durigan Junior PR cli/21688 * gdb.python/py-cmd.exp (test_python_inline_or_multiline): New procedure. Call it. --- gdb/ChangeLog | 7 +++++++ gdb/cli/cli-script.c | 21 +++++++++++++++++---- gdb/testsuite/ChangeLog | 6 ++++++ gdb/testsuite/gdb.python/py-cmd.exp | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 4 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index b47226bc..7d0f5cf 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2017-06-29 Sergio Durigan Junior + + PR cli/21688 + * cli/cli-script.c (command_name_equals_not_inline): New function. + (process_next_line): Adjust 'if' clauses for "python", "compile" + and "guile" to use command_name_equals_not_inline. + 2017-06-29 Pedro Alves * completer.c (expression_completer): Call diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c index e0e27ef..72f316f 100644 --- a/gdb/cli/cli-script.c +++ b/gdb/cli/cli-script.c @@ -900,6 +900,20 @@ command_name_equals (struct cmd_list_element *cmd, const char *name) && strcmp (cmd->name, name) == 0); } +/* Return true if NAME is the only command between COMMAND_START and + COMMAND_END. This is useful when we want to know whether the + command is inline (i.e., has arguments like 'python command1') or + is the start of a multi-line command block. */ + +static bool +command_name_equals_not_inline (const char *command_start, + const char *command_end, + const char *name) +{ + return (command_end - command_start == strlen (name) + && startswith (command_start, name)); +} + /* Given an input line P, skip the command and return a pointer to the first argument. */ @@ -997,21 +1011,20 @@ process_next_line (char *p, struct command_line **command, int parse_commands, { *command = build_command_line (commands_control, line_first_arg (p)); } - else if (command_name_equals (cmd, "python")) + else if (command_name_equals_not_inline (p_start, p_end, "python")) { /* Note that we ignore the inline "python command" form here. */ *command = build_command_line (python_control, ""); } - else if (command_name_equals (cmd, "compile")) + else if (command_name_equals_not_inline (p_start, p_end, "compile")) { /* Note that we ignore the inline "compile command" form here. */ *command = build_command_line (compile_control, ""); (*command)->control_u.compile.scope = COMPILE_I_INVALID_SCOPE; } - - else if (command_name_equals (cmd, "guile")) + else if (command_name_equals_not_inline (p_start, p_end, "guile")) { /* Note that we ignore the inline "guile command" form here. */ *command = build_command_line (guile_control, ""); diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 41c5434..51dd86d 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2017-06-29 Sergio Durigan Junior + + PR cli/21688 + * gdb.python/py-cmd.exp (test_python_inline_or_multiline): New + procedure. Call it. + 2017-06-29 Pedro Alves * gdb.base/printcmds.exp: Add tests. diff --git a/gdb/testsuite/gdb.python/py-cmd.exp b/gdb/testsuite/gdb.python/py-cmd.exp index 2dbf23ce..39bb785 100644 --- a/gdb/testsuite/gdb.python/py-cmd.exp +++ b/gdb/testsuite/gdb.python/py-cmd.exp @@ -181,6 +181,38 @@ gdb_test "complete expr_test bar\." \ "expr_test bar\.bc.*expr_test bar\.ij.*" \ "test completion through complete command" +# Test that the "python" command is correctly recognized as +# inline/multi-line when entering a sequence of commands. +# +# This proc tests PR cli/21688. The PR is not language-specific, but +# the easiest way is just to test with Python. +proc test_python_inline_or_multiline { } { + set define_cmd_not_inline { + { "if 1" " >$" "multi-line if 1" } + { "python" " >$" "multi-line python command" } + { "print ('hello')" " >$" "multi-line print" } + { "end" " >$" "multi-line first end" } + { "end" "hello\r\n" "multi-line last end" } } + + set define_cmd_inline { + { "if 1" " >$" "inline if 1" } + { "python print ('hello')" " >$" "inline python command" } + { "end" "hello\r\n" "inline end" } } + + foreach t [list $define_cmd_not_inline $define_cmd_inline] { + foreach l $t { + lassign $l command regex testmsg + gdb_test_multiple "$command" "$testmsg" { + -re "$regex" { + pass "$testmsg" + } + } + } + } +} + +test_python_inline_or_multiline + if { [readline_is_used] } { set test "complete 'expr_test bar.i'" send_gdb "expr_test bar\.i\t\t" -- 2.9.3