RFA: if/while commands in user-defined command behave differently

RFA: if/while commands in user-defined command behave differently

[Jason Molenda]

[-- Attachment #1: Type: text/plain, Size: 1358 bytes --]

 From the gdb prompt, this is accepted:

(gdb) if(3<4)
  >print "hi\n"
  >end
$1 = "hi\n"
(gdb)

But the same sequence of commands in a user-defined command will yield 
an error:

(gdb) define f
Type commands for definition of "f".
End with a line saying just "end".
 >if(3<4)
  >print "hi\n"
  >end
 >end
(gdb) f
Junk after end of expression.
(gdb)

It is hard for a user to figure out what gdb's complaining about.  
Luckily we have a really tenacious developer at Apple who tracked it 
down and filed a bug report.

Below is a patch to fix this in cli/cli-script.c, and a testsuite 
addition to gdb.base/define.exp to test that this isn't regressed.  The 
cli-script.c change alone does not change the testsuite results on x86 
Linux w/ gcc 3.4.0 & dwarf2.

I'm not clear on who needs to approve the CLI patch; Michael should 
approve the testsuite change, obviously.  OK to commit?

Jason


[gdb/ChangeLog]
2004-09-08  Jason Molenda  (jmolenda@apple.com)

         * cli/cli-script.c (read_next_line): Accept zero or more 
whitespace
         chars after 'if' or 'while' commands in user-defined commands.

[gdb/testsuite/ChangeLog]
2004-09-08  Jason Molenda  (jmolenda@apple.com)

         * gdb.base/define.exp: Two new tests to verify zero space chars
         after 'if' and 'while' commands in a user-defined command is 
correctly
         parsed.


[-- Attachment #2: pa.txt --]
[-- Type: text/plain, Size: 3949 bytes --]

Index: cli/cli-script.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-script.c,v
retrieving revision 1.23
diff -u -p -r1.23 cli-script.c
--- cli/cli-script.c	21 Apr 2004 23:52:21 -0000	1.23
+++ cli/cli-script.c	8 Sep 2004 19:19:53 -0000
@@ -1,7 +1,7 @@
 /* GDB CLI command scripting.
 
    Copyright 1986, 1987, 1988, 1989, 1990, 1991, 1992, 1993, 1994,
-   1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002 Free Software
+   1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2004 Free Software
    Foundation, Inc.
 
    This file is part of GDB.
@@ -727,9 +727,21 @@ read_next_line (struct command_line **co
   /* Check for while, if, break, continue, etc and build a new command
      line structure for them.  */
   if (p1 - p > 5 && !strncmp (p, "while", 5))
-    *command = build_command_line (while_control, p + 6);
+    {
+      char *first_arg;
+      first_arg = p + 5;
+      while (first_arg < p1 && isspace (*first_arg))
+        first_arg++;
+      *command = build_command_line (while_control, first_arg);
+    }
   else if (p1 - p > 2 && !strncmp (p, "if", 2))
-    *command = build_command_line (if_control, p + 3);
+    {
+      char *first_arg;
+      first_arg = p + 2;
+      while (first_arg < p1 && isspace (*first_arg))
+        first_arg++;
+      *command = build_command_line (if_control, first_arg);
+    }
   else if (p1 - p == 10 && !strncmp (p, "loop_break", 10))
     {
       *command = (struct command_line *)
Index: testsuite/gdb.base/define.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/define.exp,v
retrieving revision 1.4
diff -u -p -r1.4 define.exp
--- testsuite/gdb.base/define.exp	9 Dec 2003 18:19:20 -0000	1.4
+++ testsuite/gdb.base/define.exp	8 Sep 2004 19:19:53 -0000
@@ -1,4 +1,4 @@
-# Copyright 1998, 1999, 2001, 2003 Free Software Foundation, Inc.
+# Copyright 1998, 1999, 2001, 2003, 2004 Free Software Foundation, Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -200,6 +200,47 @@ gdb_expect {
   timeout {fail "(timeout) help user command: nextwhere"}
 }
 
+# Verify that the command parser doesn't require a space after an 'if'
+# command in a user defined function.
+#
+send_gdb "define ifspace\n"
+gdb_expect {
+  -re "Type commands for definition of \"ifspace\".\r\nEnd with a line saying just \"end\".\r\n>$"\
+          {send_gdb "if(3<4)\nprint \"hi there\\n\"\nend\nend\n"
+           gdb_expect {
+             -re "$gdb_prompt $"\
+                     {pass "define user command: ifspace"}
+             timeout {fail "(timeout) define user command: ifspace"}
+           }
+          }
+  -re "$gdb_prompt $"\
+          {fail "define user command: ifspace"}
+  timeout {fail "(timeout) define user command: ifspace"}
+}
+
+gdb_test "ifspace" ".*hi there.*" "test ifspace is parsed correctly"
+
+# Verify that the command parser doesn't require a space after an 'while'
+# command in a user defined function.
+#
+send_gdb "define whilespace\n"
+gdb_expect {
+  -re "Type commands for definition of \"whilespace\".\r\nEnd with a line saying just \"end\".\r\n>$"\
+          {send_gdb "set \$i=1\nwhile(\$i<2)\nset \$i=2\nprint \"hi there\\n\"\nend\nend\n"
+           gdb_expect {
+             -re "$gdb_prompt $"\
+                     {pass "define user command: whilespace"}
+             timeout {fail "(timeout) define user command: whilespace"}
+           }
+          }
+  -re "$gdb_prompt $"\
+          {fail "define user command: whilespace"}
+  timeout {fail "(timeout) define user command: whilespace"}
+}
+
+gdb_test "whilespace" ".*hi there.*" "test whilespace is parsed correctly"
+
+
 # Verify that the user can "hook" a builtin command.  We choose to
 # hook the "stop" pseudo command, and we'll define it to use a user-
 # define command.

Re: RFA: if/while commands in user-defined command behave differently

[Michael Chastain]

send_gdb / gdb_expect are old; can you can upgrade this to the modern
interface, gdb_test_multiple?  With gdb_test_multiple you don't need
the default "$gdb_prompt $" and timeout clauses.

The idea is good; it just needs gdb_test_multiple.  Just do that and
re-test and re-submit.

Michael

===

[gdb/testsuite/ChangeLog]
2004-09-08  Jason Molenda  (jmolenda@apple.com)

         * gdb.base/define.exp: Two new tests to verify zero space chars
         after 'if' and 'while' commands in a user-defined command is correctly
         parsed.

Re: RFA: if/while commands in user-defined command behave differently

[Jason Molenda]

[-- Attachment #1: Type: text/plain, Size: 1109 bytes --]


On Sep 9, 2004, at 7:34 AM, Michael Chastain wrote:

> send_gdb / gdb_expect are old; can you can upgrade this to the modern
> interface, gdb_test_multiple?  With gdb_test_multiple you don't need
> the default "$gdb_prompt $" and timeout clauses.
>
> The idea is good; it just needs gdb_test_multiple.  Just do that and
> re-test and re-submit.


OK, I switched to gdb_test_multiple, thanks for the feedback.  I 
haven't used gdb_test_multiple before, but I think this patch is 
correct.

I haven't heard anything back on the CLI change.  If the testsuite 
change is approved, should I commit that and file a PR to track the 
bug?  :-)

Jason


[gdb/ChangeLog]
2004-09-08  Jason Molenda  (jmolenda@apple.com)

         * cli/cli-script.c (read_next_line): Accept zero or more 
whitespace
         chars after 'if' or 'while' commands in user-defined commands.

[gdb/testsuite/ChangeLog]
2004-09-09  Jason Molenda  (jmolenda@apple.com)

         * gdb.base/define.exp: Two new tests to verify zero space chars
         after 'if' and 'while' commands in a user-defined command is 
correctly
         parsed.


[-- Attachment #2: pa2.txt --]
[-- Type: text/plain, Size: 3584 bytes --]

Index: cli/cli-script.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-script.c,v
retrieving revision 1.23
diff -u -p -r1.23 cli-script.c
--- cli/cli-script.c	21 Apr 2004 23:52:21 -0000	1.23
+++ cli/cli-script.c	8 Sep 2004 19:19:53 -0000
@@ -1,7 +1,7 @@
 /* GDB CLI command scripting.
 
    Copyright 1986, 1987, 1988, 1989, 1990, 1991, 1992, 1993, 1994,
-   1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002 Free Software
+   1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2004 Free Software
    Foundation, Inc.
 
    This file is part of GDB.
@@ -727,9 +727,21 @@ read_next_line (struct command_line **co
   /* Check for while, if, break, continue, etc and build a new command
      line structure for them.  */
   if (p1 - p > 5 && !strncmp (p, "while", 5))
-    *command = build_command_line (while_control, p + 6);
+    {
+      char *first_arg;
+      first_arg = p + 5;
+      while (first_arg < p1 && isspace (*first_arg))
+        first_arg++;
+      *command = build_command_line (while_control, first_arg);
+    }
   else if (p1 - p > 2 && !strncmp (p, "if", 2))
-    *command = build_command_line (if_control, p + 3);
+    {
+      char *first_arg;
+      first_arg = p + 2;
+      while (first_arg < p1 && isspace (*first_arg))
+        first_arg++;
+      *command = build_command_line (if_control, first_arg);
+    }
   else if (p1 - p == 10 && !strncmp (p, "loop_break", 10))
     {
       *command = (struct command_line *)
Index: define.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/define.exp,v
retrieving revision 1.4
diff -u -p -r1.4 define.exp
--- define.exp	9 Dec 2003 18:19:20 -0000	1.4
+++ define.exp	10 Sep 2004 00:36:36 -0000
@@ -1,4 +1,4 @@
-# Copyright 1998, 1999, 2001, 2003 Free Software Foundation, Inc.
+# Copyright 1998, 1999, 2001, 2003, 2004 Free Software Foundation, Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -200,6 +200,40 @@ gdb_expect {
   timeout {fail "(timeout) help user command: nextwhere"}
 }
 
+# Verify that the command parser doesn't require a space after an 'if'
+# command in a user defined function.
+#
+gdb_test_multiple "define ifnospace" "define user command: ifnospace" \
+{
+  -re "Type commands for definition of \"ifnospace\".\r\nEnd with a line saying just \"end\".\r\n>$" \
+    {
+      gdb_test_multiple "if(3<4)\nprint \"hi there\\n\"\nend\nend" "send body of ifnospace"  \
+        {
+         -re "$gdb_prompt $"\
+                 {pass "define user command: ifspace"}
+        }
+    }
+}
+
+gdb_test "ifnospace" ".*hi there.*" "test ifnospace is parsed correctly"
+
+# Verify that the command parser doesn't require a space after an 'while'
+# command in a user defined function.
+#
+gdb_test_multiple "define whilenospace" "define user command: whilenospace" \
+{
+  -re "Type commands for definition of \"whilenospace\".\r\nEnd with a line saying just \"end\".\r\n>$" \
+    {
+      gdb_test_multiple "set \$i=1\nwhile(\$i<2)\nset \$i=2\nprint \"hi there\\n\"\nend\nend" "send body of whilenospace" \
+         {
+           -re "$gdb_prompt $" \
+                   {pass "define user command: whilenospace"}
+         }
+    }
+}
+
+gdb_test "whilenospace" ".*hi there.*" "test whilenospace is parsed correctly"
+
 # Verify that the user can "hook" a builtin command.  We choose to
 # hook the "stop" pseudo command, and we'll define it to use a user-
 # define command.

Re: RFA: if/while commands in user-defined command behave differently

[Michael Chastain]

Jason Molenda <jmolenda@apple.com> wrote:
> OK, I switched to gdb_test_multiple, thanks for the feedback.  I 
> haven't used gdb_test_multiple before, but I think this patch is 
> correct.

Just assert that you re-tested it, and it's approved.
(I'm a weird combination of anal-retentive and trusting).

> I haven't heard anything back on the CLI change.  If the testsuite 
> change is approved, should I commit that and file a PR to track the 
> bug?  :-)

Actually, I like it that way.  But you've been working on GDB longer
than I have, so I leave it to your judgement when/if to file a PR.

Michael

===

2004-09-09  Jason Molenda  (jmolenda@apple.com)

         * gdb.base/define.exp: Two new tests to verify zero space chars
         after 'if' and 'while' commands in a user-defined command is 
	correctly parsed.

Re: RFA: if/while commands in user-defined command behave differently

[Jason Molenda]

On Sep 9, 2004, at 5:55 PM, Michael Chastain wrote:

> Jason Molenda <jmolenda@apple.com> wrote:
>> OK, I switched to gdb_test_multiple, thanks for the feedback.  I
>> haven't used gdb_test_multiple before, but I think this patch is
>> correct.
>
> Just assert that you re-tested it, and it's approved.
> (I'm a weird combination of anal-retentive and trusting).

Yes, sorry, x86 Linux gcc 3.4.0 dwarf2, no failures (with the patch to 
cli-script.c, of course :)

>> I haven't heard anything back on the CLI change.  If the testsuite
>> change is approved, should I commit that and file a PR to track the
>> bug?  :-)
>
> Actually, I like it that way.  But you've been working on GDB longer
> than I have, so I leave it to your judgement when/if to file a PR.

Cool.  I have to take off in a few minutes, but I'll file a PR and 
commit the testsuite addition tomorrow.

Thanks again for the help,


J

Re: RFA: if/while commands in user-defined command behave differently

[Daniel Jacobowitz]

On Wed, Sep 08, 2004 at 12:29:54PM -0700, Jason Molenda wrote:
> [gdb/ChangeLog]
> 2004-09-08  Jason Molenda  (jmolenda@apple.com)
> 
>         * cli/cli-script.c (read_next_line): Accept zero or more 
> whitespace
>         chars after 'if' or 'while' commands in user-defined commands.

OK, with the approved version of the test.

-- 
Daniel Jacobowitz

Re: [Committed] RFA: if/while commands in user-defined command behave differently

[Jason Molenda]

[-- Attachment #1: Type: text/plain, Size: 123 bytes --]


On Sep 9, 2004, at 6:19 PM, Daniel Jacobowitz wrote:

> OK, with the approved version of the test.


Thanks, committed.



[-- Attachment #2: pa3.txt --]
[-- Type: text/plain, Size: 4893 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.6355
diff -u -p -r1.6355 ChangeLog
--- ChangeLog	8 Sep 2004 21:58:16 -0000	1.6355
+++ ChangeLog	10 Sep 2004 23:10:50 -0000
@@ -1,3 +1,8 @@
+2004-09-10  Jason Molenda  (jmolenda@apple.com)
+
+        * cli/cli-script.c (read_next_line): Accept zero or more whitespace
+        chars after 'if' or 'while' commands in user-defined commands.
+
 2004-09-08  Jim Blandy  <jimb@redhat.com>
 
         Fix bug reported and analyzed by Olivier Crete:
Index: cli/cli-script.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-script.c,v
retrieving revision 1.23
diff -u -p -r1.23 cli-script.c
--- cli/cli-script.c	21 Apr 2004 23:52:21 -0000	1.23
+++ cli/cli-script.c	10 Sep 2004 23:10:50 -0000
@@ -1,7 +1,7 @@
 /* GDB CLI command scripting.
 
    Copyright 1986, 1987, 1988, 1989, 1990, 1991, 1992, 1993, 1994,
-   1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002 Free Software
+   1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2004 Free Software
    Foundation, Inc.
 
    This file is part of GDB.
@@ -727,9 +727,21 @@ read_next_line (struct command_line **co
   /* Check for while, if, break, continue, etc and build a new command
      line structure for them.  */
   if (p1 - p > 5 && !strncmp (p, "while", 5))
-    *command = build_command_line (while_control, p + 6);
+    {
+      char *first_arg;
+      first_arg = p + 5;
+      while (first_arg < p1 && isspace (*first_arg))
+        first_arg++;
+      *command = build_command_line (while_control, first_arg);
+    }
   else if (p1 - p > 2 && !strncmp (p, "if", 2))
-    *command = build_command_line (if_control, p + 3);
+    {
+      char *first_arg;
+      first_arg = p + 2;
+      while (first_arg < p1 && isspace (*first_arg))
+        first_arg++;
+      *command = build_command_line (if_control, first_arg);
+    }
   else if (p1 - p == 10 && !strncmp (p, "loop_break", 10))
     {
       *command = (struct command_line *)
Index: testsuite/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/ChangeLog,v
retrieving revision 1.1027
diff -u -p -r1.1027 ChangeLog
--- testsuite/ChangeLog	8 Sep 2004 22:00:46 -0000	1.1027
+++ testsuite/ChangeLog	10 Sep 2004 23:10:50 -0000
@@ -1,3 +1,9 @@
+2004-09-10  Jason Molenda  (jmolenda@apple.com)
+
+        * gdb.base/define.exp: Two new tests to verify zero space chars
+        after 'if' and 'while' commands in a user-defined command is correctly
+        parsed.
+
 2004-09-08  Andrew Cagney  <cagney@gnu.org>
 
 	* gdb.base/signals.exp (signal_tests_1): Delete.  Merge signal
Index: testsuite/gdb.base/define.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/define.exp,v
retrieving revision 1.4
diff -u -p -r1.4 define.exp
--- testsuite/gdb.base/define.exp	9 Dec 2003 18:19:20 -0000	1.4
+++ testsuite/gdb.base/define.exp	10 Sep 2004 23:10:51 -0000
@@ -1,4 +1,4 @@
-# Copyright 1998, 1999, 2001, 2003 Free Software Foundation, Inc.
+# Copyright 1998, 1999, 2001, 2003, 2004 Free Software Foundation, Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -200,6 +200,40 @@ gdb_expect {
   timeout {fail "(timeout) help user command: nextwhere"}
 }
 
+# Verify that the command parser doesn't require a space after an 'if'
+# command in a user defined function.
+#
+gdb_test_multiple "define ifnospace" "define user command: ifnospace" \
+{
+  -re "Type commands for definition of \"ifnospace\".\r\nEnd with a line saying just \"end\".\r\n>$" \
+    {
+      gdb_test_multiple "if(3<4)\nprint \"hi there\\n\"\nend\nend" "send body of ifnospace"  \
+        {
+         -re "$gdb_prompt $"\
+                 {pass "define user command: ifnospace"}
+        }
+    }
+}
+
+gdb_test "ifnospace" ".*hi there.*" "test ifnospace is parsed correctly"
+
+# Verify that the command parser doesn't require a space after an 'while'
+# command in a user defined function.
+#
+gdb_test_multiple "define whilenospace" "define user command: whilenospace" \
+{
+  -re "Type commands for definition of \"whilenospace\".\r\nEnd with a line saying just \"end\".\r\n>$" \
+    {
+      gdb_test_multiple "set \$i=1\nwhile(\$i<2)\nset \$i=2\nprint \"hi there\\n\"\nend\nend" "send body of whilenospace" \
+         {
+           -re "$gdb_prompt $" \
+                   {pass "define user command: whilenospace"}
+         }
+    }
+}
+
+gdb_test "whilenospace" ".*hi there.*" "test whilenospace is parsed correctly"
+
 # Verify that the user can "hook" a builtin command.  We choose to
 # hook the "stop" pseudo command, and we'll define it to use a user-
 # define command.