This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] PR cli/21688: Fix multi-line/inline command differentiation
- From: Simon Marchi <simon dot marchi at polymtl dot ca>
- To: Sergio Durigan Junior <sergiodj at redhat dot com>
- Cc: GDB Patches <gdb-patches at sourceware dot org>, Jerome Guitton <guitton at adacore dot com>
- Date: Thu, 29 Jun 2017 21:08:35 +0200
- Subject: Re: [PATCH] PR cli/21688: Fix multi-line/inline command differentiation
- Authentication-results: sourceware.org; auth=none
- References: <20170629020527.468-1-sergiodj@redhat.com>
On 2017-06-29 04:05, Sergio Durigan Junior wrote:
This bug is a regression caused by the following commit:
604c4576fdcfc4e7c28f569b3748a1b6b4e0dbd4 is the first bad commit
commit 604c4576fdcfc4e7c28f569b3748a1b6b4e0dbd4
Author: Jerome Guitton <guitton@adacore.com>
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.
Hi Sergio,
Thanks for the fix and the test! I have a few comments below.
gdb/ChangeLog:
2017-06-29 Sergio Durigan Junior <sergiodj@redhat.com>
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 <sergiodj@redhat.com>
PR cli/21688
* gdb.python/py-cmd.exp (test_pr21688): New procedure. Call it.
---
gdb/ChangeLog | 7 +++++++
gdb/cli/cli-script.c | 21 +++++++++++++++++----
gdb/testsuite/ChangeLog | 5 +++++
gdb/testsuite/gdb.python/py-cmd.exp | 30
++++++++++++++++++++++++++++++
4 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a82026f..194cda8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2017-06-29 Sergio Durigan Junior <sergiodj@redhat.com>
+
+ 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-28 Pedro Alves <palves@redhat.com>
* command.h: Include "common/scoped_restore.h".
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"))
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).
{
/* 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 b7462a5..ef46a5d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2017-06-29 Sergio Durigan Junior <sergiodj@redhat.com>
+
+ PR cli/21688
+ * gdb.python/py-cmd.exp (test_pr21688): New procedure. Call it.
+
2017-06-28 Doug Gilmore <Doug.Gilmore@imgtec.com>
PR gdb/21337
diff --git a/gdb/testsuite/gdb.python/py-cmd.exp
b/gdb/testsuite/gdb.python/py-cmd.exp
index 2dbf23ce..052afa4 100644
--- a/gdb/testsuite/gdb.python/py-cmd.exp
+++ b/gdb/testsuite/gdb.python/py-cmd.exp
@@ -181,6 +181,36 @@ gdb_test "complete expr_test bar\." \
"expr_test bar\.bc.*expr_test bar\.ij.*" \
"test completion through complete command"
+# Testing PR cli/21688. This is not language-specific, but the
+# easiest way is just to test with Python.
+proc test_pr21688 { } {
I am not a fan of naming procs and documenting things solely based on PR
numbers, it's cryptic and requires to go check the web page to know what
this is for. I'd prefer a short description (e.g. Test that the
"python" command is correctly recognized as an inline or multi-line
command when entering a sequence of commands, something like that) and
an appropriate name. Mentioning the PR in the comment is still good
though, if the reader wants to know the context in which this was added.
+ set define_cmd_not_inline {
+ { "if 1" " >$" }
+ { "python" " >$" }
+ { "print ('hello')" " >$" }
+ { "end" " >$" }
+ { "end" "hello\r\n" } }
+
+ set define_cmd_inline {
+ { "if 1" " >$" }
+ { "python print ('hello')" " >$" }
+ { "end" "hello\r\n" } }
+
+ foreach t [list $define_cmd_not_inline $define_cmd_inline] {
+ foreach l $t {
+ foreach { command regex } $l {
I didn't understand this last foreach at first, but IIUC it's for
unpacking $l? An alternative that might be clearer is lassign:
lassign $l command regex
+ gdb_test_multiple "$command" "$command" {
Watch out, this will make some test names that end with parenthesis, and
a few of them will be non-unique.
+ -re "$regex" {
+ pass "$command"
+ }
+ }
+ }
+ }
+ }
+}
+
+test_pr21688
+
if { [readline_is_used] } {
set test "complete 'expr_test bar.i'"
send_gdb "expr_test bar\.i\t\t"
Thanks,
Simon