public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 2/5] Further cleanup/modernization of gdb.base/commands.exp
  2016-11-09  0:19 [PATCH v2 0/5] Support an "unlimited" number of user-defined arguments Pedro Alves
  2016-11-09  0:19 ` [PATCH v2 1/5] gdb/testsuite: Introduce "proc_with_prefix" Pedro Alves
  2016-11-09  0:19 ` [PATCH v2 3/5] Fix PR 20559 - "eval" command and $arg0...$arg9/$argc substitution Pedro Alves
@ 2016-11-09  0:19 ` Pedro Alves
  2016-11-09  3:02   ` Simon Marchi
  2016-11-09  0:25 ` [PATCH v2 5/5] Support an "unlimited" number of user-defined arguments Pedro Alves
  2016-11-09  0:26 ` [PATCH v2 4/5] Test user-defined gdb commands and arguments stack Pedro Alves
  4 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2016-11-09  0:19 UTC (permalink / raw)
  To: gdb-patches

 - Use multi_line for matching multi-line GDB output.

 - Add a multi_line_input variant of multi_line to build GDB input and
   use it throughout.

   (The two changes above make the tests much more readable, IMO.)

 - Remove gdb_stop_suppressing_tests uses.

 - tighten a few regexps.

 - Replace send_gdb/gdb_expect with gdb_test_multiple and simplify,
   making pass/fail messages the same.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.base/commands.exp (runto_or_return): New
	procedure.
	(gdbvar_simple_if_test, gdbvar_simple_while_test)
	(gdbvar_complex_if_while_test, progvar_simple_if_test)
	(progvar_simple_while_test, progvar_complex_if_while_test)
	(if_while_breakpoint_command_test)
	(infrun_breakpoint_command_test, breakpoint_command_test)
	(user_defined_command_test, watchpoint_command_test)
	(test_command_prompt_position, redefine_hook_test)
	(redefine_backtrace_test): Use multi_line_input and multi_line.
	* lib/gdb.exp (multi_line_input): New procedure.
---
 gdb/testsuite/gdb.base/commands.exp | 333 +++++++++++++++++++++++-------------
 gdb/testsuite/lib/gdb.exp           |   9 +
 2 files changed, 226 insertions(+), 116 deletions(-)

diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index fe2c23a..b6c8f9e 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -23,16 +23,41 @@ if { [prepare_for_testing commands.exp commands run.c {debug additional_flags=-D
     return -1
 }
 
+# Run to FUNCTION.  If that fails, issue a FAIL and make the caller
+# return.
+
+proc runto_or_return {function} {
+    if { ![runto factorial] } {
+	fail "cannot run to $function"
+	return -code return
+    }
+}
+
 proc_with_prefix gdbvar_simple_if_test {} {
     global gdb_prompt
 
     gdb_test_no_output "set \$foo = 0" "set foo"
     # All this test should do is print 0xdeadbeef once.
-    gdb_test "if \$foo == 1\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
-	    "\\\$\[0-9\]* = 0xdeadbeef" "#1"
+    gdb_test \
+	[multi_line_input \
+	     "if \$foo == 1" \
+	     "  p/x 0xfeedface" \
+	     "else" \
+	     "  p/x 0xdeadbeef" \
+	     "end"] \
+	"\\\$\[0-9\]* = 0xdeadbeef" \
+	"#1"
+
     # All this test should do is print 0xfeedface once.
-    gdb_test "if \$foo == 0\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
-	    "\\\$\[0-9\]* = 0xfeedface" "#2"
+    gdb_test \
+	[multi_line_input \
+	     "if \$foo == 0" \
+	     "  p/x 0xfeedface" \
+	     "else" \
+	     "  p/x 0xdeadbeef" \
+	     "end"] \
+	"\\\$\[0-9\]* = 0xfeedface" \
+	"#2"
 }
 
 proc_with_prefix gdbvar_simple_while_test {} {
@@ -40,74 +65,133 @@ proc_with_prefix gdbvar_simple_while_test {} {
 
     gdb_test_no_output "set \$foo = 5" "set foo"
     # This test should print 0xfeedface five times.
-    gdb_test "while \$foo > 0\np/x 0xfeedface\nset \$foo -= 1\nend" \
-	    "\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "#1"
+    gdb_test \
+	[multi_line_input \
+	     "while \$foo > 0" \
+	     "  p/x 0xfeedface" \
+	     "  set \$foo -= 1" \
+	     "end"] \
+	[multi_line \
+	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface"] \
+	"#1"
 }
 
 proc_with_prefix gdbvar_complex_if_while_test {} {
     global gdb_prompt
 
-    gdb_test_no_output "set \$foo = 4" \
-	"set foo"
+    gdb_test_no_output "set \$foo = 4" "set foo"
     # This test should alternate between 0xdeadbeef and 0xfeedface two times.
-    gdb_test "while \$foo > 0\nset \$foo -= 1\nif \(\$foo % 2\) == 1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend" \
-	    "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "#1"
+    gdb_test \
+	[multi_line_input \
+	     "while \$foo > 0" \
+	     "  set \$foo -= 1" \
+	     "  if \(\$foo % 2\) == 1" \
+	     "    p/x 0xdeadbeef" \
+	     "  else" \
+	     "    p/x 0xfeedface" \
+	     "  end" \
+	     "end"] \
+	[multi_line \
+	     "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface"] \
+	"#1"
 }
 
 proc_with_prefix progvar_simple_if_test {} {
     global gdb_prompt
 
-    if { ![runto factorial] } then { gdb_suppress_tests; }
+    runto_or_return factorial
+
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=5" ".*" "set value to 5"
+    gdb_test "p value=5" " = 5" "set value to 5"
     # All this test should do is print 0xdeadbeef once.
-    gdb_test "if value == 1\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
-	    "\\\$\[0-9\]* = 0xdeadbeef" \
-	    "#1"
+    gdb_test \
+	[multi_line_input \
+	     "if value == 1" \
+	     "  p/x 0xfeedface" \
+	     "else" \
+	     "  p/x 0xdeadbeef" \
+	     "end"] \
+	"\\\$\[0-9\]* = 0xdeadbeef" \
+	"#1"
+
     # All this test should do is print 0xfeedface once.
-    gdb_test "if value == 5\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
-	    "\\\$\[0-9\]* = 0xfeedface" \
-	    "#2"
-    gdb_stop_suppressing_tests
+    gdb_test \
+	[multi_line_input \
+	     "if value == 5" \
+	     "  p/x 0xfeedface" \
+	     "else" \
+	     "  p/x 0xdeadbeef" \
+	     "end"] \
+	"\\\$\[0-9\]* = 0xfeedface" \
+	"#2"
 }
 
 proc_with_prefix progvar_simple_while_test {} {
     global gdb_prompt
 
-    if { ![runto factorial] } then { gdb_suppress_tests }
+    runto_or_return factorial
+
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=5" ".*" "set value to 5"
+    gdb_test "p value=5" " = 5" "set value to 5"
     # This test should print 0xfeedface five times.
-    gdb_test "while value > 0\np/x 0xfeedface\nset value -= 1\nend" \
-	    "\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "#1"
-    gdb_stop_suppressing_tests
+    gdb_test \
+	[multi_line_input \
+	     "while value > 0" \
+	     "  p/x 0xfeedface" \
+	     "  set value -= 1" \
+	     "end"] \
+	[multi_line \
+	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface"] \
+	"#1"
 }
 
 proc_with_prefix progvar_complex_if_while_test {} {
     global gdb_prompt
 
-    if { ![runto factorial] } then { gdb_suppress_tests }
+    runto_or_return factorial
+
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=4" ".*" "set value to 4"
-    # This test should alternate between 0xdeadbeef and 0xfeedface two times.
-    gdb_test "while value > 0\nset value -= 1\nif \(value % 2\) == 1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend" \
-	    "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "#1"
-    gdb_stop_suppressing_tests
+    gdb_test "p value=4" " = 4" "set value to 4"
+    # This test should alternate between 0xdeadbeef and 0xfeedface two
+    # times.
+    gdb_test \
+	[multi_line_input \
+	     "while value > 0" \
+	     "  set value -= 1" \
+	     "  if \(value % 2\) == 1" \
+	     "    p/x 0xdeadbeef" \
+	     "  else" \
+	     "    p/x 0xfeedface" \
+	     "  end" \
+	     "end"] \
+	[multi_line \
+	     "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface"] \
+	"#1"
 }
 
 proc_with_prefix if_while_breakpoint_command_test {} {
+    runto_or_return factorial
 
-    if { ![runto factorial] } then { gdb_suppress_tests }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=5" ".*" "set value to 5"
+    gdb_test "p value=5" " = 5" "set value to 5"
     delete_breakpoints
     gdb_test "break factorial" "Breakpoint.*at.*" "break factorial"
 
@@ -118,14 +202,28 @@ proc_with_prefix if_while_breakpoint_command_test {} {
     }
 
     # This test should alternate between 0xdeadbeef and 0xfeedface two times.
-    gdb_test "while value > 0\nset value -= 1\nif \(value % 2\) == 1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend\nend" \
-	    "" \
-	    "commands part 2"
-    gdb_test "continue" \
-	    "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "#1"
+    gdb_test \
+	[multi_line_input \
+	     "while value > 0" \
+	     "  set value -= 1" \
+	     "  if \(value % 2\) == 1" \
+	     "    p/x 0xdeadbeef" \
+	     "  else" \
+	     "    p/x 0xfeedface" \
+	     "  end" \
+	     "end" \
+	     "end"] \
+	"" \
+	"commands part 2"
+    gdb_test \
+	"continue" \
+	[multi_line \
+	     "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface"] \
+	"#1"
     gdb_test "info break" "while.*set.*if.*p/x.*else.*p/x.*end.*"
-    gdb_stop_suppressing_tests
 }
 
 # Test that we can run the inferior from breakpoint commands.
@@ -135,11 +233,11 @@ proc_with_prefix if_while_breakpoint_command_test {} {
 # subsection "Breakpoint command lists".
 
 proc_with_prefix infrun_breakpoint_command_test {} {
+    runto_or_return factorial
 
-    if { ![runto factorial] } then { gdb_suppress_tests }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=6" ".*" "set value to 6"
+    gdb_test "p value=6" " = 6" "set value to 6"
     delete_breakpoints
     gdb_test "break factorial if value == 5" "Breakpoint.*at.*"
 
@@ -159,24 +257,26 @@ proc_with_prefix infrun_breakpoint_command_test {} {
 
     gdb_test "continue" \
 	"Continuing.*.*.*Breakpoint \[0-9\]*, factorial \\(value=5\\).*at.*\[0-9\]*\[      \]*if \\(value > 1\\) \{.*\[0-9\]*\[      \]*value \\*= factorial \\(value - 1\\);.*"
-
-    gdb_stop_suppressing_tests
 }
 
 proc_with_prefix breakpoint_command_test {} {
+    runto_or_return factorial
 
-    if { ![runto factorial] } then { gdb_suppress_tests; }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=6" ".*" "set value to 6"
+    gdb_test "p value=6" " = 6" "set value to 6"
     delete_breakpoints
     gdb_test "break factorial" "Breakpoint.*at.*"
-    gdb_test "commands\nprintf \"Now the value is %d\\n\", value\nend" \
-	"End with.*" "commands"
+    gdb_test \
+	[multi_line_input \
+	     "commands" \
+	     "  printf \"Now the value is %d\\n\", value" \
+	     "end"] \
+	"End with.*" \
+	"commands"
     gdb_test "continue" \
 	    "Breakpoint \[0-9\]*, factorial.*Now the value is 5"
     gdb_test "print value" " = 5"
-    gdb_stop_suppressing_tests
 }
 
 # Test a simple user defined command (with arguments)
@@ -192,13 +292,28 @@ proc_with_prefix user_defined_command_test {} {
     }
 
     # This test should alternate between 0xdeadbeef and 0xfeedface two times.
-    gdb_test "while \$arg0 > 0\nset \$arg0 -= 1\nif \(\$arg0 % 2\) == 1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend\nend" \
-	    "" \
-	    "enter commands"
-
-    gdb_test "mycommand \$foo" \
-	    "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "execute user-defined command"
+    gdb_test \
+	[multi_line_input \
+	     "while \$arg0 > 0" \
+	     "  set \$arg0 -= 1" \
+	     "  if \(\$arg0 % 2\) == 1" \
+	     "    p/x 0xdeadbeef" \
+	     "  else" \
+	     "    p/x 0xfeedface" \
+	     "  end" \
+	     "end" \
+	     "end"] \
+	"" \
+	"enter commands"
+
+    gdb_test \
+	"mycommand \$foo" \
+	[multi_line \
+	     "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface"] \
+	"execute user-defined command"
    gdb_test "show user mycommand" \
 	"  while \\\$arg0.*set.*    if \\\(\\\$arg0.*p/x.*    else\[^\n\].*p/x.*    end\[^\n\].*  end\[^\n\].*" \
 	   "display user command"
@@ -233,7 +348,8 @@ proc_with_prefix watchpoint_command_test {} {
 	gdb_test_no_output "set can-use-hw-watchpoints 0" ""
     }
 
-    if { ![runto factorial] } then { return }
+    runto_or_return factorial
+
     delete_breakpoints
 
     # Verify that we can create a watchpoint, and give it a commands
@@ -299,59 +415,42 @@ proc_with_prefix watchpoint_command_test {} {
 proc_with_prefix test_command_prompt_position {} {
     global gdb_prompt
 
-    if { ![runto factorial] } then { gdb_suppress_tests; }
+    runto_or_return factorial
+
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
     delete_breakpoints
     gdb_test "break factorial" "Breakpoint.*at.*"
     gdb_test "p value=5" ".*" "set value to 5"
     # All this test should do is print 0xdeadbeef once.
-    gdb_test "if value == 1\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
-	    "\\\$\[0-9\]* = 0xdeadbeef" \
-	    "if test"
-    
+    gdb_test \
+	[multi_line_input \
+	     "if value == 1" \
+	     "  p/x 0xfeedface" \
+	     "else" \
+	     "  p/x 0xdeadbeef" \
+	     "end"] \
+	"\\\$\[0-9\]* = 0xdeadbeef" \
+	"if test"
+
     # Now let's test for the correct position of the '>' in gdb's
     # prompt for commands.  It should be at the beginning of the line,
     # and not after one space.
 
-    send_gdb "commands\n"
-    gdb_expect {
-	-re "Type commands.*End with.*\[\r\n\]>$" { 
-	    send_gdb "printf \"Now the value is %d\\n\", value\n"
-	    gdb_expect {
+    set test "> OK"
+    gdb_test_multiple "commands" $test {
+	-re "Type commands.*End with.*\[\r\n\]>$" {
+	    gdb_test_multiple "printf \"Now the value is %d\\n\", value" $test {
 		-re "^printf.*value\r\n>$" {
-		    send_gdb "end\n"
-		    gdb_expect {
+		    gdb_test_multiple "end" $test {
 			-re "^end\r\n$gdb_prompt $" { 
-			    pass "> OK"
-			}
-			-re ".*$gdb_prompt $" { 
-			    fail "some other message"
-			}
-			timeout  { 
-			    fail "(timeout) 1"
+			    pass $test
 			}
 		    }
 		}
-		-re "^ >$" { fail "> not OK" }
-		-re ".*$gdb_prompt $"   { 
-		    fail "wrong message"
-		}
-		timeout    { 
-		    fail "(timeout) 2"
-		}
 	    }
 	}
-	-re "Type commands.*End with.*\[\r\n\] >$" { 
-	    fail "prompt not OK"
-	}
-	-re ".*$gdb_prompt $" { 
-	    fail "commands"
-	}
-	timeout { fail "(timeout) 3" }
     }
-
-    gdb_stop_suppressing_tests
 }
 
 
@@ -693,13 +792,20 @@ proc_with_prefix error_clears_commands_left {} {
 proc_with_prefix redefine_hook_test {} {
     global gdb_prompt
 
-    gdb_test "define one\nend" \
-      "" \
-      "define one"
+    gdb_test \
+	[multi_line_input \
+	     "define one"\
+	     "end"] \
+	"" \
+	"define one"
 
-    gdb_test "define hook-one\necho hibob\\n\nend" \
-      "" \
-      "define hook-one"
+    gdb_test \
+	[multi_line_input \
+	     "define hook-one" \
+	     "echo hibob\\n" \
+	     "end"] \
+	"" \
+	"define hook-one"
 
     set test "redefine one"
     gdb_test_multiple "define one" $test {
@@ -713,13 +819,9 @@ proc_with_prefix redefine_hook_test {} {
 	}
     }
 
-    gdb_test "end" \
-	    "" \
-	    "enter commands for one redefinition"
+    gdb_test "end" "" "enter commands for one redefinition"
 
-    gdb_test "one" \
-	    "hibob" \
-	    "execute one command"
+    gdb_test "one" "hibob" "execute one command"
 }
 
 proc_with_prefix redefine_backtrace_test {} {
@@ -737,16 +839,15 @@ proc_with_prefix redefine_backtrace_test {} {
 	}
     }
 
-    gdb_test "echo hibob\\n\nend" \
-	    "" \
-	    "enter commands"
+    gdb_test \
+	[multi_line_input \
+	     "echo hibob\\n" \
+	     "end"] \
+	"" \
+	"enter commands"
 
-    gdb_test "backtrace" \
-	    "hibob" \
-	    "execute backtrace command"
-    gdb_test "bt" \
-	    "hibob" \
-	    "execute bt command"
+    gdb_test "backtrace" "hibob" "execute backtrace command"
+    gdb_test "bt" "hibob" "execute bt command"
 }
 
 gdbvar_simple_if_test
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 735ed11..1ba2509 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -6005,5 +6005,14 @@ proc multi_line { args } {
     return [join $args "\r\n"]
 }
 
+# Similar to the above, but while multi_line is meant to be used to
+# match GDB output, this one is meant to be used to build strings to
+# send as GDB input.
+
+proc multi_line_input { args } {
+    return [join $args "\n"]
+}
+
+
 # Always load compatibility stuff.
 load_lib future.exp
-- 
2.5.5

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 3/5] Fix PR 20559 - "eval" command and $arg0...$arg9/$argc substitution
  2016-11-09  0:19 [PATCH v2 0/5] Support an "unlimited" number of user-defined arguments Pedro Alves
  2016-11-09  0:19 ` [PATCH v2 1/5] gdb/testsuite: Introduce "proc_with_prefix" Pedro Alves
@ 2016-11-09  0:19 ` Pedro Alves
  2016-11-09  0:19 ` [PATCH v2 2/5] Further cleanup/modernization of gdb.base/commands.exp Pedro Alves
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2016-11-09  0:19 UTC (permalink / raw)
  To: gdb-patches

It'd be handy to be able to iterate over command arguments in
user-defined commands, in order to support optional arguments
($arg0..$argN).

I thought I could make it work with "eval", but alas, it doesn't work
currently.  E.g., with:

 define test
   set $i = 0
   while $i < $argc
     eval "print $arg%d", $i
     set $i = $i + 1
   end
 end

we get:

 (gdb) test 1
 $1 = void
 (gdb) test 1 2 3
 $2 = void
 $3 = void
 $4 = void
 (gdb)

The problem is that "eval" doesn't do user-defined command arguments
substitution after expanding its own argument.  This patch fixes that,
which makes the example above work:

 (gdb) test 1
 $1 = 1
 (gdb) test 1 2 3
 $2 = 1
 $3 = 2
 $4 = 3
 (gdb)

New test included, similar the above, but also exercises expanding
$argc.

I think this is likely to simplify many scripts out there, so I'm
adding an example to the manual and mentioning it in NEWS as well.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	PR cli/20559
	* NEWS: Mention "eval" expands user-defined command arguments.
	* cli/cli-script.c (execute_control_command): Adjust to rename.
	(insert_args): Rename to ...
	(insert_user_defined_cmd_args): ... this, and make extern.
	* cli/cli-script.h (insert_user_defined_cmd_args): New
	declaration.
	* printcmd.c: Include "cli/cli-script.h".
	(eval_command): Call insert_user_defined_cmd_args.

gdb/doc/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	PR cli/20559
	* gdb.texinfo (Define): Add example of using "eval" to process a
	variable number of arguments.
	(Output) <eval>: Add anchor.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	PR cli/20559
	* gdb.base/commands.exp (user_defined_command_args_eval): New
	procedure.
	(top level): Call it.
---
 gdb/doc/gdb.texinfo                 | 16 ++++++++++++++++
 gdb/NEWS                            | 12 ++++++++++++
 gdb/cli/cli-script.c                | 19 ++++++++-----------
 gdb/cli/cli-script.h                |  6 ++++++
 gdb/printcmd.c                      |  3 +++
 gdb/testsuite/gdb.base/commands.exp | 31 +++++++++++++++++++++++++++++++
 6 files changed, 76 insertions(+), 11 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index df548dc..5311912 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -24096,6 +24096,21 @@ define adder
 end
 @end smallexample
 
+Combining with the @code{eval} command (@pxref{eval}) makes it easier
+to process a variable number of arguments:
+
+@smallexample
+define adder
+  set $i = 0
+  set $sum = 0
+  while $i < $argc
+    eval "set $sum = $sum + $arg%d", $i
+    set $i = $i + 1
+  end
+  print $sum
+end
+@end smallexample
+
 @table @code
 
 @kindex define
@@ -24526,6 +24541,7 @@ Here's an example of printing DFP types using the above conversion letters:
 printf "D32: %Hf - D64: %Df - D128: %DDf\n",1.2345df,1.2E10dd,1.2E1dl
 @end smallexample
 
+@anchor{eval}
 @kindex eval
 @item eval @var{template}, @var{expressions}@dots{}
 Convert the values of one or more @var{expressions} under the control of
diff --git a/gdb/NEWS b/gdb/NEWS
index a6b1282..790629b 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -27,6 +27,18 @@
 
 * Support for Java programs compiled with gcj has been removed.
 
+* The "eval" command now expands user-defined command arguments.
+
+  This makes it easier to process a variable number of arguments:
+
+   define mycommand
+     set $i = 0
+     while $i < $argc
+       eval "print $arg%d", $i
+       set $i = $i + 1
+     end
+   end
+
 * New targets
 
 Synopsys ARC			arc*-*-elf32
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index ce4d8cb..15947a9 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -41,8 +41,6 @@ recurse_read_control_structure (char * (*read_next_line_func) (void),
 				void (*validator)(char *, void *),
 				void *closure);
 
-static std::string insert_args (const char *line);
-
 static struct cleanup * setup_user_args (char *p);
 
 static char *read_next_line (void);
@@ -455,7 +453,7 @@ execute_control_command (struct command_line *cmd)
     case simple_control:
       {
 	/* A simple command, execute it and return.  */
-	std::string new_line = insert_args (cmd->line);
+	std::string new_line = insert_user_defined_cmd_args (cmd->line);
 	execute_command (&new_line[0], 0);
 	ret = cmd->control_type;
 	break;
@@ -486,7 +484,7 @@ execute_control_command (struct command_line *cmd)
 	print_command_trace (buffer);
 
 	/* Parse the loop control expression for the while statement.  */
-	std::string new_line = insert_args (cmd->line);
+	std::string new_line = insert_user_defined_cmd_args (cmd->line);
 	expression_up expr = parse_expression (new_line.c_str ());
 
 	ret = simple_control;
@@ -551,7 +549,7 @@ execute_control_command (struct command_line *cmd)
 	print_command_trace (buffer);
 
 	/* Parse the conditional for the if statement.  */
-	std::string new_line = insert_args (cmd->line);
+	std::string new_line = insert_user_defined_cmd_args (cmd->line);
 	expression_up expr = parse_expression (new_line.c_str ());
 
 	current = NULL;
@@ -591,7 +589,7 @@ execute_control_command (struct command_line *cmd)
       {
 	/* Breakpoint commands list, record the commands in the
 	   breakpoint's command list and return.  */
-	std::string new_line = insert_args (cmd->line);
+	std::string new_line = insert_user_defined_cmd_args (cmd->line);
 	ret = commands_from_control_command (new_line.c_str (), cmd);
 	break;
       }
@@ -782,13 +780,12 @@ locate_arg (const char *p)
   return NULL;
 }
 
-/* Insert the user defined arguments stored in user_arg into the $arg
-   arguments found in line.  */
+/* See cli-script.h.  */
 
-static std::string
-insert_args (const char *line)
+std::string
+insert_user_defined_cmd_args (const char *line)
 {
-  /* If we are not in a user-defined function, treat $argc, $arg0, et
+  /* If we are not in a user-defined command, treat $argc, $arg0, et
      cetera as normal convenience variables.  */
   if (user_args == NULL)
     return line;
diff --git a/gdb/cli/cli-script.h b/gdb/cli/cli-script.h
index 7db6e82..8920d44 100644
--- a/gdb/cli/cli-script.h
+++ b/gdb/cli/cli-script.h
@@ -53,6 +53,12 @@ extern struct cleanup *
 
 extern void execute_user_command (struct cmd_list_element *c, char *args);
 
+/* If we're in a user-defined command, replace any $argc/$argN
+   reference found in LINE with the arguments that were passed to the
+   command.  Otherwise, treat $argc/$argN as normal convenience
+   variables.  */
+extern std::string insert_user_defined_cmd_args (const char *line);
+
 /* Exported to top.c */
 
 extern void print_command_trace (const char *cmd);
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 23de57c..4004ee7 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -45,6 +45,7 @@
 #include "charset.h"
 #include "arch-utils.h"
 #include "cli/cli-utils.h"
+#include "cli/cli-script.h"
 #include "format.h"
 #include "source.h"
 
@@ -2724,6 +2725,8 @@ eval_command (char *arg, int from_tty)
 
   std::string expanded = ui_file_as_string (ui_out);
 
+  expanded = insert_user_defined_cmd_args (expanded.c_str ());
+
   execute_command (&expanded[0], from_tty);
 
   do_cleanups (cleanups);
diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index b6c8f9e..0ac2267 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -340,6 +340,36 @@ proc_with_prefix user_defined_command_test {} {
 	"display user-defined empty command"
 }
 
+# Test that "eval" in a user-defined command expands $argc/$argN.
+
+proc_with_prefix user_defined_command_args_eval {} {
+    global gdb_prompt
+
+    gdb_test_multiple "define command_args_eval" \
+	"define command_args_eval" {
+	    -re "End with"  {
+		pass "define"
+	    }
+	}
+
+    # Make a command that constructs references to $argc and $argN via
+    # eval.
+    gdb_test \
+	[multi_line \
+	     {eval "printf \"argc = %%d,\", $arg%c", 'c'} \
+	     {set $i = 0} \
+	     {while $i < $argc} \
+	     {  eval "printf \" %%d\", $arg%d", $i} \
+	     {  set $i = $i + 1} \
+	     {end} \
+	     {printf "\n"} \
+	     {end}] \
+	"" \
+	"enter commands"
+
+    gdb_test "command_args_eval 1 2 3" "argc = 3, 1 2 3" "execute command"
+}
+
 proc_with_prefix watchpoint_command_test {} {
     global gdb_prompt
 
@@ -860,6 +890,7 @@ if_while_breakpoint_command_test
 infrun_breakpoint_command_test
 breakpoint_command_test
 user_defined_command_test
+user_defined_command_args_eval
 watchpoint_command_test
 test_command_prompt_position
 deprecated_command_test
-- 
2.5.5

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 0/5] Support an "unlimited" number of user-defined arguments
@ 2016-11-09  0:19 Pedro Alves
  2016-11-09  0:19 ` [PATCH v2 1/5] gdb/testsuite: Introduce "proc_with_prefix" Pedro Alves
                   ` (4 more replies)
  0 siblings, 5 replies; 21+ messages in thread
From: Pedro Alves @ 2016-11-09  0:19 UTC (permalink / raw)
  To: gdb-patches

As I mentioned at
<https://sourceware.org/ml/gdb-patches/2016-11/msg00173.html>, the
patch at <https://sourceware.org/ml/gdb-patches/2016-10/msg00576.html>
is now expanded into a series of its own.  Hence the "v2".

Patches #1 - #4 are all new and are preparatory patches for patch #5,
which is is the same patch as the old one, but simplified a bit
further by assuming C++11, and now with a test included.

Patch #3 stands on its own right.  The lifting of the limit of number
of arguments makes it a necessity rather than just a nice-to-have,
though.  This patch includes docs/NEWS changes.

Pedro Alves (5):
  gdb/testsuite: Introduce "proc_with_prefix"
  Further cleanup/modernization of gdb.base/commands.exp
  Fix PR 20559 - "eval" command and $arg0...$arg9/$argc substitution
  Test user-defined gdb commands and arguments stack
  Support an "unlimited" number of user-defined arguments

 gdb/doc/gdb.texinfo                 |  22 +-
 gdb/NEWS                            |  15 +
 gdb/cli/cli-script.c                | 202 ++++++------
 gdb/cli/cli-script.h                |   6 +
 gdb/printcmd.c                      |   3 +
 gdb/testsuite/gdb.base/commands.exp | 621 ++++++++++++++++++++++++------------
 gdb/testsuite/lib/gdb.exp           |  17 +
 7 files changed, 593 insertions(+), 293 deletions(-)

-- 
2.5.5

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 1/5] gdb/testsuite: Introduce "proc_with_prefix"
  2016-11-09  0:19 [PATCH v2 0/5] Support an "unlimited" number of user-defined arguments Pedro Alves
@ 2016-11-09  0:19 ` Pedro Alves
  2016-11-09  1:56   ` Simon Marchi
                     ` (2 more replies)
  2016-11-09  0:19 ` [PATCH v2 3/5] Fix PR 20559 - "eval" command and $arg0...$arg9/$argc substitution Pedro Alves
                   ` (3 subsequent siblings)
  4 siblings, 3 replies; 21+ messages in thread
From: Pedro Alves @ 2016-11-09  0:19 UTC (permalink / raw)
  To: gdb-patches

While adding new tests to gdb.base/commands.exp, I noticed that the
file includes a bunch of individual testcases split into their own
procedures, and that none have ever been adjusted to use
with_test_prefix.  Instead, each gdb_test/gdb_test_multiple/etc
invocation takes care of including the procedure name in the test
message, in order to make sure test messages are unique.

Simon convinced me that using the procedure name as prefix is not that
bad of an idea:
  https://sourceware.org/ml/gdb-patches/2016-10/msg00020.html

This commit adds an IMO simpler alternative to
with_test_prefix_procname added by that patch -- a new
"proc_with_prefix" convenience proc that is meant to be used in place
of "proc", and then uses it in commands.exp.  Procedures defined with
this automatically run their bodies under with_test_prefix $proc_name.

Here's a sample of the resulting gdb.sum diff:

 [...]
 -PASS: gdb.base/commands.exp: break factorial #3
 -PASS: gdb.base/commands.exp: set value to 5 in test_command_prompt_position
 -PASS: gdb.base/commands.exp: if test in test_command_prompt_position
 -PASS: gdb.base/commands.exp: > OK in test_command_prompt_position
 +PASS: gdb.base/commands.exp: test_command_prompt_position: break factorial
 +PASS: gdb.base/commands.exp: test_command_prompt_position: set value to 5
 +PASS: gdb.base/commands.exp: test_command_prompt_position: if test
 +PASS: gdb.base/commands.exp: test_command_prompt_position: > OK
 [...]

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.base/commands.exp (gdbvar_simple_if_test)
	(gdbvar_simple_while_test, gdbvar_complex_if_while_test)
	(progvar_simple_if_test, progvar_simple_while_test)
	(progvar_complex_if_while_test, if_while_breakpoint_command_test)
	(infrun_breakpoint_command_test, breakpoint_command_test)
	(user_defined_command_test, watchpoint_command_test)
	(test_command_prompt_position, deprecated_command_test)
	(bp_deleted_in_command, temporary_breakpoint_commands)
	(stray_arg0_test, source_file_with_indented_comment)
	(recursive_source_test, if_commands_test)
	(error_clears_commands_left, redefine_hook_test)
	(redefine_backtrace_test): Use proc_with_prefix.
	* lib/gdb.exp (proc_with_prefix): New proc.
---
 gdb/testsuite/gdb.base/commands.exp | 224 +++++++++++++++++-------------------
 gdb/testsuite/lib/gdb.exp           |   8 ++
 2 files changed, 115 insertions(+), 117 deletions(-)

diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index 11d6db7..fe2c23a 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -23,111 +23,108 @@ if { [prepare_for_testing commands.exp commands run.c {debug additional_flags=-D
     return -1
 }
 
-proc gdbvar_simple_if_test {} {
+proc_with_prefix gdbvar_simple_if_test {} {
     global gdb_prompt
 
-    gdb_test_no_output "set \$foo = 0" "set foo in gdbvar_simple_if_test"
+    gdb_test_no_output "set \$foo = 0" "set foo"
     # All this test should do is print 0xdeadbeef once.
     gdb_test "if \$foo == 1\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
-	    "\\\$\[0-9\]* = 0xdeadbeef" "gdbvar_simple_if_test #1"
+	    "\\\$\[0-9\]* = 0xdeadbeef" "#1"
     # All this test should do is print 0xfeedface once.
     gdb_test "if \$foo == 0\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
-	    "\\\$\[0-9\]* = 0xfeedface" "gdbvar_simple_if_test #2"
+	    "\\\$\[0-9\]* = 0xfeedface" "#2"
 }
 
-proc gdbvar_simple_while_test {} {
+proc_with_prefix gdbvar_simple_while_test {} {
     global gdb_prompt
 
-    gdb_test_no_output "set \$foo = 5" "set foo in gdbvar_simple_while_test"
+    gdb_test_no_output "set \$foo = 5" "set foo"
     # This test should print 0xfeedface five times.
     gdb_test "while \$foo > 0\np/x 0xfeedface\nset \$foo -= 1\nend" \
 	    "\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "gdbvar_simple_while_test #1"
+	    "#1"
 }
 
-proc gdbvar_complex_if_while_test {} {
+proc_with_prefix gdbvar_complex_if_while_test {} {
     global gdb_prompt
 
     gdb_test_no_output "set \$foo = 4" \
-	"set foo in gdbvar complex_if_while_test"
+	"set foo"
     # This test should alternate between 0xdeadbeef and 0xfeedface two times.
     gdb_test "while \$foo > 0\nset \$foo -= 1\nif \(\$foo % 2\) == 1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend" \
 	    "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "gdbvar_complex_if_while_test #1"
+	    "#1"
 }
 
-proc progvar_simple_if_test {} {
+proc_with_prefix progvar_simple_if_test {} {
     global gdb_prompt
 
     if { ![runto factorial] } then { gdb_suppress_tests; }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=5" ".*" "set value to 5 in progvar_simple_if_test #1"
+    gdb_test "p value=5" ".*" "set value to 5"
     # All this test should do is print 0xdeadbeef once.
     gdb_test "if value == 1\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
 	    "\\\$\[0-9\]* = 0xdeadbeef" \
-	    "progvar_simple_if_test #1"
+	    "#1"
     # All this test should do is print 0xfeedface once.
     gdb_test "if value == 5\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
 	    "\\\$\[0-9\]* = 0xfeedface" \
-	    "progvar_simple_if_test #2"
+	    "#2"
     gdb_stop_suppressing_tests
 }
 
-proc progvar_simple_while_test {} {
+proc_with_prefix progvar_simple_while_test {} {
     global gdb_prompt
 
     if { ![runto factorial] } then { gdb_suppress_tests }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=5" ".*" "set value to 5 in progvar_simple_if_test #2"
+    gdb_test "p value=5" ".*" "set value to 5"
     # This test should print 0xfeedface five times.
     gdb_test "while value > 0\np/x 0xfeedface\nset value -= 1\nend" \
 	    "\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "progvar_simple_while_test #1"
+	    "#1"
     gdb_stop_suppressing_tests
 }
 
-proc progvar_complex_if_while_test {} {
+proc_with_prefix progvar_complex_if_while_test {} {
     global gdb_prompt
 
     if { ![runto factorial] } then { gdb_suppress_tests }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=4" ".*" "set value to 4 in progvar_simple_if_test"
+    gdb_test "p value=4" ".*" "set value to 4"
     # This test should alternate between 0xdeadbeef and 0xfeedface two times.
     gdb_test "while value > 0\nset value -= 1\nif \(value % 2\) == 1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend" \
 	    "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "progvar_complex_if_while_test #1"
+	    "#1"
     gdb_stop_suppressing_tests
 }
 
-proc if_while_breakpoint_command_test {} {
+proc_with_prefix if_while_breakpoint_command_test {} {
 
     if { ![runto factorial] } then { gdb_suppress_tests }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=5" ".*" "set value to 5 in progvar_simple_if_test"
+    gdb_test "p value=5" ".*" "set value to 5"
     delete_breakpoints
-    gdb_test "break factorial" "Breakpoint.*at.*" "break factorial #1"
+    gdb_test "break factorial" "Breakpoint.*at.*" "break factorial"
 
-    gdb_test_multiple "commands" \
-	"commands in if_while_breakpoint_command_test" {
-	    -re "End with" {
-		pass "commands in if_while_breakpoint_command_test"
-	    }
+    gdb_test_multiple "commands" "commands" {
+	-re "End with" {
+	    pass "commands"
 	}
+    }
 
     # This test should alternate between 0xdeadbeef and 0xfeedface two times.
     gdb_test "while value > 0\nset value -= 1\nif \(value % 2\) == 1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend\nend" \
 	    "" \
-	    "commands part 2 in if_while_breakpoint_command_test"
+	    "commands part 2"
     gdb_test "continue" \
 	    "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "if_while_breakpoint_command_test #1"
-   gdb_test "info break" \
-	   "while.*set.*if.*p/x.*else.*p/x.*end.*" \
-	   "info break in if_while_breakpoint_command_test"
+	    "#1"
+    gdb_test "info break" "while.*set.*if.*p/x.*else.*p/x.*end.*"
     gdb_stop_suppressing_tests
 }
 
@@ -137,12 +134,12 @@ proc if_while_breakpoint_command_test {} {
 # shall be ignored.  See the gdb manual, "Break Commands",
 # subsection "Breakpoint command lists".
 
-proc infrun_breakpoint_command_test {} {
+proc_with_prefix infrun_breakpoint_command_test {} {
 
     if { ![runto factorial] } then { gdb_suppress_tests }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=6" ".*" "set value to 6 in progvar_simple_if_test #1"
+    gdb_test "p value=6" ".*" "set value to 6"
     delete_breakpoints
     gdb_test "break factorial if value == 5" "Breakpoint.*at.*"
 
@@ -150,89 +147,85 @@ proc infrun_breakpoint_command_test {} {
 # to get around a synchronization problem in expect.
 # part1: issue the gdb command "commands"
 # part2: send the list of commands
-    gdb_test_multiple "commands" \
-	"commands in infrun_breakpoint_command_test #1" {
-	    -re "End with" {
-		pass "commands in infrun_breakpoint_command_test #1"
-	    }
+
+    set test "commands #1"
+    gdb_test_multiple "commands" $test {
+	-re "End with" {
+	    pass $test
 	}
+    }
     gdb_test "step\nstep\nstep\nstep\nend" "" \
-	"commands in infrun_breakpoint_command_test #2"
+	"commands #2"
 
     gdb_test "continue" \
-	"Continuing.*.*.*Breakpoint \[0-9\]*, factorial \\(value=5\\).*at.*\[0-9\]*\[      \]*if \\(value > 1\\) \{.*\[0-9\]*\[      \]*value \\*= factorial \\(value - 1\\);.*" \
-	"continue in infrun_breakpoint_command_test"
+	"Continuing.*.*.*Breakpoint \[0-9\]*, factorial \\(value=5\\).*at.*\[0-9\]*\[      \]*if \\(value > 1\\) \{.*\[0-9\]*\[      \]*value \\*= factorial \\(value - 1\\);.*"
 
     gdb_stop_suppressing_tests
 }
 
-proc breakpoint_command_test {} {
+proc_with_prefix breakpoint_command_test {} {
 
     if { ![runto factorial] } then { gdb_suppress_tests; }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=6" ".*" "set value to 6 in progvar_simple_if_test #2"
+    gdb_test "p value=6" ".*" "set value to 6"
     delete_breakpoints
-    gdb_test "break factorial" "Breakpoint.*at.*" "break factorial #2"
+    gdb_test "break factorial" "Breakpoint.*at.*"
     gdb_test "commands\nprintf \"Now the value is %d\\n\", value\nend" \
-	"End with.*" "commands in breakpoint_command_test"
+	"End with.*" "commands"
     gdb_test "continue" \
-	    "Breakpoint \[0-9\]*, factorial.*Now the value is 5" \
-	"continue in breakpoint_command_test"
-    gdb_test "print value" " = 5" "print value in breakpoint_command_test"
+	    "Breakpoint \[0-9\]*, factorial.*Now the value is 5"
+    gdb_test "print value" " = 5"
     gdb_stop_suppressing_tests
 }
 
 # Test a simple user defined command (with arguments)
-proc user_defined_command_test {} {
+proc_with_prefix user_defined_command_test {} {
     global gdb_prompt
 
-    gdb_test_no_output "set \$foo = 4" \
-	"set foo in user_defined_command_test"
+    gdb_test_no_output "set \$foo = 4" "set foo"
 
-    gdb_test_multiple "define mycommand" \
-	"define mycommand in user_defined_command_test" {
-	    -re "End with"  {
-		pass "define mycommand in user_defined_command_test"
-	    }
+    gdb_test_multiple "define mycommand" "define mycommand" {
+	-re "End with"  {
+	    pass "define mycommand"
 	}
+    }
 
     # This test should alternate between 0xdeadbeef and 0xfeedface two times.
     gdb_test "while \$arg0 > 0\nset \$arg0 -= 1\nif \(\$arg0 % 2\) == 1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend\nend" \
 	    "" \
-	    "enter commands in user_defined_command_test"
+	    "enter commands"
 
     gdb_test "mycommand \$foo" \
 	    "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "execute user defined command in user_defined_command_test"
+	    "execute user-defined command"
    gdb_test "show user mycommand" \
 	"  while \\\$arg0.*set.*    if \\\(\\\$arg0.*p/x.*    else\[^\n\].*p/x.*    end\[^\n\].*  end\[^\n\].*" \
-	   "display user command in user_defined_command_test"
+	   "display user command"
 
     # Create and test a user-defined command with an empty body.
-    gdb_test_multiple "define myemptycommand" \
-	"define myemptycommand in user_defined_command_test" {
-	    -re "End with"  {
-		pass "define myemptycommand in user_defined_command_test"
-	    }
+    gdb_test_multiple "define myemptycommand" "define myemptycommand" {
+	-re "End with"  {
+	    pass "define myemptycommand"
 	}
+    }
     gdb_test "end" \
 	"" \
 	"end definition of user-defined command with empty body"
 
     gdb_test_no_output "myemptycommand" \
-	"execute user-defined empty command in user_defined_command_test"
+	"execute user-defined empty command"
 
     gdb_test "show user" \
 	"User command \"myemptycommand.*" \
-	"display empty command in command list in user_defined_command_test"
+	"display empty command in command list"
 
     gdb_test "show user myemptycommand" \
 	"User command \"myemptycommand.*" \
-	"display user-defined empty command in user_defined_command_test"
+	"display user-defined empty command"
 }
 
-proc watchpoint_command_test {} {
+proc_with_prefix watchpoint_command_test {} {
     global gdb_prompt
 
     # Disable hardware watchpoints if necessary.
@@ -303,19 +296,19 @@ proc watchpoint_command_test {} {
    }
 }
 
-proc test_command_prompt_position {} {
+proc_with_prefix test_command_prompt_position {} {
     global gdb_prompt
 
     if { ![runto factorial] } then { gdb_suppress_tests; }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
     delete_breakpoints
-    gdb_test "break factorial" "Breakpoint.*at.*" "break factorial #3"
-    gdb_test "p value=5" ".*" "set value to 5 in test_command_prompt_position"
+    gdb_test "break factorial" "Breakpoint.*at.*"
+    gdb_test "p value=5" ".*" "set value to 5"
     # All this test should do is print 0xdeadbeef once.
     gdb_test "if value == 1\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
 	    "\\\$\[0-9\]* = 0xdeadbeef" \
-	    "if test in test_command_prompt_position"
+	    "if test"
     
     # Now let's test for the correct position of the '>' in gdb's
     # prompt for commands.  It should be at the beginning of the line,
@@ -330,32 +323,32 @@ proc test_command_prompt_position {} {
 		    send_gdb "end\n"
 		    gdb_expect {
 			-re "^end\r\n$gdb_prompt $" { 
-			    pass "> OK in test_command_prompt_position" 
+			    pass "> OK"
 			}
 			-re ".*$gdb_prompt $" { 
-			    fail "some other message in test_command_prompt_position" 
+			    fail "some other message"
 			}
 			timeout  { 
-			    fail "(timeout) 1 in test_command_prompt_position"
+			    fail "(timeout) 1"
 			}
 		    }
 		}
-		-re "^ >$" { fail "> not OK in test_command_prompt_position" }
+		-re "^ >$" { fail "> not OK" }
 		-re ".*$gdb_prompt $"   { 
-		    fail "wrong message in test_command_prompt_position" 
+		    fail "wrong message"
 		}
 		timeout    { 
-		    fail "(timeout) 2 in test_command_prompt_position " 
+		    fail "(timeout) 2"
 		}
 	    }
 	}
 	-re "Type commands.*End with.*\[\r\n\] >$" { 
-	    fail "prompt not OK in test_command_prompt_position" 
+	    fail "prompt not OK"
 	}
 	-re ".*$gdb_prompt $" { 
-	    fail "commands in test_command_prompt_position" 
+	    fail "commands"
 	}
-	timeout { fail "(timeout) 3 commands in test_command_prompt_position" }
+	timeout { fail "(timeout) 3" }
     }
 
     gdb_stop_suppressing_tests
@@ -363,7 +356,7 @@ proc test_command_prompt_position {} {
 
 
 
-proc deprecated_command_test {} {
+proc_with_prefix deprecated_command_test {} {
     gdb_test "maintenance deprecate blah" "Can't find command.*" \
           "tried to deprecate non-existing command"
 
@@ -397,7 +390,7 @@ proc deprecated_command_test {} {
 	    "deprecate with no arguments"
 }
 
-proc bp_deleted_in_command_test {} {
+proc_with_prefix bp_deleted_in_command_test {} {
     global gdb_prompt
 
     delete_breakpoints
@@ -405,12 +398,11 @@ proc bp_deleted_in_command_test {} {
     # Create a breakpoint, and associate a command-list to it, with
     # one command that deletes this breakpoint.
     gdb_test "break factorial" \
-             "Breakpoint \[0-9\]+ at .*: file .*run.c, line \[0-9\]+\." \
-             "breakpoint in bp_deleted_in_command_test"
+             "Breakpoint \[0-9\]+ at .*: file .*run.c, line \[0-9\]+\."
     
-    gdb_test_multiple "commands" "begin commands in bp_deleted_in_command_test" {
+    gdb_test_multiple "commands" "begin commands" {
       -re "Type commands for breakpoint.*>$" {
-          pass "begin commands in bp_deleted_in_command_test"
+          pass "begin commands"
       }
     }
     gdb_test_multiple "silent" "add silent command" {
@@ -442,7 +434,7 @@ proc bp_deleted_in_command_test {} {
     gdb_test "" "factorial command-list executed.*" "run factorial until breakpoint"
 }
 
-proc temporary_breakpoint_commands {} {
+proc_with_prefix temporary_breakpoint_commands {} {
     global gdb_prompt
 
     delete_breakpoints
@@ -452,12 +444,12 @@ proc temporary_breakpoint_commands {} {
     # breakpoint is hit.
     gdb_test "tbreak factorial" \
 	    "Temporary breakpoint \[0-9\]+ at .*: file .*run.c, line \[0-9\]+\." \
-	    "breakpoint in temporary_breakpoint_commands"
-    
+	    "breakpoint"
+
     gdb_test_multiple "commands" \
 	"begin commands in bp_deleted_in_command_test" {
 	    -re "Type commands for breakpoint.*>$" {
-		pass "begin commands in bp_deleted_in_command_test"
+		pass "begin commands"
 	    }
 	}
     gdb_test_multiple "silent" "add silent tbreak command" {
@@ -487,26 +479,26 @@ proc temporary_breakpoint_commands {} {
 
 # Test that GDB can handle $arg0 outside of user functions without
 # crashing.
-proc stray_arg0_test { } {
+proc_with_prefix stray_arg0_test { } {
     gdb_test "print \$arg0" \
 	"\\\$\[0-9\]* = void" \
-	"stray_arg0_test #1"
+	"#1"
 
     gdb_test "if 1 == 1\nprint \$arg0\nend" \
 	"\\\$\[0-9\]* = void" \
-	"stray_arg0_test #2"
+	"#2"
 
     gdb_test "print \$arg0 = 1" \
 	"\\\$\[0-9\]* = 1" \
-	"stray_arg0_test #3"
+	"#3"
 
     gdb_test "print \$arg0" \
 	"\\\$\[0-9\]* = 1" \
-	"stray_arg0_test #4"
+	"#4"
 }
 
 # Test that GDB is able to source a file with an indented comment.
-proc source_file_with_indented_comment {} {
+proc_with_prefix source_file_with_indented_comment {} {
     set file1 [standard_output_file file1]
 
     set fd [open "$file1" w]
@@ -517,12 +509,12 @@ end
 echo Done!\n}
     close $fd
 
-    gdb_test "source $file1" "Done!" "source file with indented comment"
+    gdb_test "source $file1" "Done!" "source file"
 }
 
 # Test that GDB can handle arguments when sourcing files recursively.
 # If the arguments are overwritten with ####### then the test has failed.
-proc recursive_source_test {} {
+proc_with_prefix recursive_source_test {} {
     set file1 [standard_output_file file1]
     set file2 [standard_output_file file2]
     set file3 [standard_output_file file3]
@@ -550,7 +542,7 @@ end"
 
     gdb_test "source $file1" \
 	"1: <<<qwerty>>>\[\r\n]+in file3\[\r\n]+2: <<<qwerty>>>" \
-	"recursive source test"
+	"source file"
 
     file delete $file1
     file delete $file2
@@ -575,10 +567,10 @@ proc gdb_test_no_prompt { command result msg } {
     return 0
 }
 
-proc if_commands_test {} {
+proc_with_prefix if_commands_test {} {
     global gdb_prompt
 
-    gdb_test_no_output "set \$tem = 1" "set \$tem in if_commands_test"
+    gdb_test_no_output "set \$tem = 1" "set \$tem"
 
     set test "if_commands_test 1"
     gdb_test_no_prompt "if \$tem == 2" { >} $test
@@ -633,7 +625,7 @@ proc if_commands_test {} {
 # Verify an error during "commands" commands execution will prevent any other
 # "commands" from other breakpoints at the same location to be executed.
 
-proc error_clears_commands_left {} {
+proc_with_prefix error_clears_commands_left {} {
     set test "hook-stop 1"
     gdb_test_multiple {define hook-stop} $test {
 	-re "End with a line saying just \"end\"\\.\r\n>$" {
@@ -698,7 +690,7 @@ proc error_clears_commands_left {} {
     gdb_test {echo idle\n} "\r\nidle" "no cmd2"
 }
 
-proc redefine_hook_test {} {
+proc_with_prefix redefine_hook_test {} {
     global gdb_prompt
 
     gdb_test "define one\nend" \
@@ -709,30 +701,28 @@ proc redefine_hook_test {} {
       "" \
       "define hook-one"
 
-    gdb_test_multiple "define one" "redefine one" {
+    set test "redefine one"
+    gdb_test_multiple "define one" $test {
 	-re "Redefine command .one.. .y or n. $" {
 	    send_gdb "y\n"
 	    exp_continue
 	}
 
 	-re "End with"  {
-	    pass "define one in redefine_hook_test"
-	}
-        default {
-	    fail "(timeout or eof) define one in redefine_hook_test"
+	    pass $test
 	}
     }
 
     gdb_test "end" \
 	    "" \
-	    "enter commands for one redefinition in redefine_hook_test"
+	    "enter commands for one redefinition"
 
     gdb_test "one" \
 	    "hibob" \
-	    "execute one command in redefine_hook_test"
+	    "execute one command"
 }
 
-proc redefine_backtrace_test {} {
+proc_with_prefix redefine_backtrace_test {} {
     global gdb_prompt
 
     gdb_test_multiple "define backtrace" "define backtrace" {
@@ -749,14 +739,14 @@ proc redefine_backtrace_test {} {
 
     gdb_test "echo hibob\\n\nend" \
 	    "" \
-	    "enter commands in redefine_backtrace_test"
+	    "enter commands"
 
     gdb_test "backtrace" \
 	    "hibob" \
-	    "execute backtrace command in redefine_backtrace_test"
+	    "execute backtrace command"
     gdb_test "bt" \
 	    "hibob" \
-	    "execute bt command in redefine_backtrace_test"
+	    "execute bt command"
 }
 
 gdbvar_simple_if_test
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 7d9b198..735ed11 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1918,6 +1918,14 @@ proc foreach_with_prefix {var list body} {
     }
 }
 
+# Like TCL's native proc, but defines a procedure that wraps its body
+# within 'with_test_prefix "$proc_name" { ... }'.
+proc proc_with_prefix {name arguments body} {
+    # Define the advertised proc.
+    proc $name $arguments [list with_test_prefix $name $body]
+}
+
+
 # Run BODY in the context of the caller.  After BODY is run, the variables
 # listed in VARS will be reset to the values they had before BODY was run.
 #
-- 
2.5.5

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 5/5] Support an "unlimited" number of user-defined arguments
  2016-11-09  0:19 [PATCH v2 0/5] Support an "unlimited" number of user-defined arguments Pedro Alves
                   ` (2 preceding siblings ...)
  2016-11-09  0:19 ` [PATCH v2 2/5] Further cleanup/modernization of gdb.base/commands.exp Pedro Alves
@ 2016-11-09  0:25 ` Pedro Alves
  2016-11-09  0:26 ` [PATCH v2 4/5] Test user-defined gdb commands and arguments stack Pedro Alves
  4 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2016-11-09  0:25 UTC (permalink / raw)
  To: gdb-patches

New in v2:

  - A few adjustments / simplifications were possible now that we
    require C++11:

    . Use std::unique_ptr to make the user_args_stack std::vector own
      its elements:

       static std::vector<std::unique_ptr<user_args>> user_args_stack;

    . use vector::emplace_back to construct elements directly in the
      corresponding vectors.

    . use std::to_string instead of adding a gdb::to_string
      replacement.

  - Now includes a test.

Docs/NEWS are unchanged from v1 and have already been approved.

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I recently wrote a user-defined command that could benefit from
supporting an unlimited number of arguments:

 http://palves.net/list-active-signal-handlers-with-gdb/

E.g., 'info signal-dispositions 1 2 3 4 5 6 7 8 9 10 11'

However, we currently only support up to 10 arguments passed to
user-defined commands ($arg0..$arg9).

I can't find a good reason for that, other than "old code with hard
coded limits".  This patch removes that limit and modernizes the code
along the way:

  - Makes the user_args struct a real C++ class that uses std::vector
    for storage.

  - Removes the "next" pointer from within user_args and uses a
    std::vector to maintain a stack instead.

  - Adds a new RAII-based scoped_user_args_level class to help
    push/pop user args in the stack instead of using a cleanup.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* NEWS: Mention that user commands now accept an unlimited number
	of arguments.
	* cli/cli-script.c: Include <vector>.
	(struct string_view): New type.
	(MAXUSERARGS): Delete.
	(struct user_args): Now a C++ class.
	(user_args_stack): New.
	(struct scoped_user_args_level): New type.
	(execute_user_command): Use scoped_user_args_level.
	(arg_cleanup): Delete.
	(setup_user_args): Deleted, and refactored as ...
	(user_args::user_args): ... this new constructor.  Limit of number
	of arguments removed.
	(insert_user_defined_cmd_args): Defer to user_args_stack.
	(user_args::insert_args): New, bits based on old
	insert_user_defined_cmd_args with limit of number of arguments
	eliminated.

gdb/doc/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.texinfo (User-defined Commands): Limit on number of
	arguments passed to user-defined commands removed; update.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.base/commands.exp (user_defined_command_manyargs_test): New
	procedure.
	(top level): Call it.
---
 gdb/doc/gdb.texinfo                 |   6 +-
 gdb/NEWS                            |   3 +
 gdb/cli/cli-script.c                | 183 ++++++++++++++++++++----------------
 gdb/testsuite/gdb.base/commands.exp |  44 +++++++++
 4 files changed, 152 insertions(+), 84 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5311912..52d0b0f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -24057,9 +24057,9 @@ files.
 @cindex arguments, to user-defined commands
 A @dfn{user-defined command} is a sequence of @value{GDBN} commands to
 which you assign a new name as a command.  This is done with the
-@code{define} command.  User commands may accept up to 10 arguments
+@code{define} command.  User commands may accept an unlimited number of arguments
 separated by whitespace.  Arguments are accessed within the user command
-via @code{$arg0@dots{}$arg9}.  A trivial example:
+via @code{$arg0@dots{}$argN}.  A trivial example:
 
 @smallexample
 define adder
@@ -24083,7 +24083,7 @@ functions calls.
 @cindex argument count in user-defined commands
 @cindex how many arguments (user-defined commands)
 In addition, @code{$argc} may be used to find out how many arguments have
-been passed.  This expands to a number in the range 0@dots{}10.
+been passed.
 
 @smallexample
 define adder
diff --git a/gdb/NEWS b/gdb/NEWS
index 790629b..26697cd 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -27,6 +27,9 @@
 
 * Support for Java programs compiled with gcj has been removed.
 
+* User commands now accept an unlimited number of arguments.
+  Previously, only up to 10 was accepted.
+
 * The "eval" command now expands user-defined command arguments.
 
   This makes it easier to process a variable number of arguments:
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 15947a9..b3ba20a 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -33,6 +33,8 @@
 #include "interps.h"
 #include "compile/compile.h"
 
+#include <vector>
+
 /* Prototypes for local functions.  */
 
 static enum command_control_type
@@ -41,8 +43,6 @@ recurse_read_control_structure (char * (*read_next_line_func) (void),
 				void (*validator)(char *, void *),
 				void *closure);
 
-static struct cleanup * setup_user_args (char *p);
-
 static char *read_next_line (void);
 
 /* Level of control structure when reading.  */
@@ -54,24 +54,68 @@ static int command_nest_depth = 1;
 /* This is to prevent certain commands being printed twice.  */
 static int suppress_next_print_command_trace = 0;
 
+/* A non-owning slice of a string.  */
+
+struct string_view
+{
+  string_view (const char *str_, size_t len_)
+    : str (str_), len (len_)
+  {}
+
+  const char *str;
+  size_t len;
+};
+
 /* Structure for arguments to user defined functions.  */
-#define MAXUSERARGS 10
-struct user_args
+
+class user_args
+{
+public:
+  /* Save the command line and store the locations of arguments passed
+     to the user defined function.  */
+  explicit user_args (const char *line);
+
+  /* Insert the stored user defined arguments into the $arg arguments
+     found in LINE.  */
+  std::string insert_args (const char *line);
+
+private:
+  /* Disable copy/assignment.  (Since the elements of A point inside
+     COMMAND, copying would need to reconstruct the A vector in the
+     new copy.)  */
+  user_args (const user_args &);
+  user_args &operator= (const user_args &);
+
+  /* It is necessary to store a copy of the command line to ensure
+     that the arguments are not overwritten before they are used.  */
+  std::string m_command_line;
+
+  /* The arguments.  Each element points inside M_COMMAND_LINE.  */
+  std::vector<string_view> m_args;
+};
+
+/* The stack of arguments passed to user defined functions.  We need a
+   stack because user-defined functions can call other user-defined
+   functions.  */
+static std::vector<std::unique_ptr<user_args>> user_args_stack;
+
+/* An RAII-base class used to push/pop args on the user args
+   stack.  */
+struct scoped_user_args_level
+{
+  /* Parse the command line and push the arguments in the user args
+     stack.  */
+  explicit scoped_user_args_level (const char *line)
   {
-    struct user_args *next;
-    /* It is necessary to store a malloced copy of the command line to
-       ensure that the arguments are not overwritten before they are
-       used.  */
-    char *command;
-    struct
-      {
-	char *arg;
-	int len;
-      }
-    a[MAXUSERARGS];
-    int count;
+    user_args_stack.emplace_back (new user_args (line));
+  }
+
+  /* Pop the current user arguments from the stack.  */
+  ~scoped_user_args_level ()
+  {
+    user_args_stack.pop_back ();
   }
- *user_args;
+};
 
 \f
 /* Return non-zero if TYPE is a multi-line command (i.e., is terminated
@@ -346,7 +390,6 @@ do_restore_user_call_depth (void * call_depth)
     in_user_command = 0;
 }
 
-
 void
 execute_user_command (struct cmd_list_element *c, char *args)
 {
@@ -362,12 +405,12 @@ execute_user_command (struct cmd_list_element *c, char *args)
     /* Null command */
     return;
 
-  old_chain = setup_user_args (args);
+  scoped_user_args_level push_user_args (args);
 
   if (++user_call_depth > max_user_call_depth)
     error (_("Max user call depth exceeded -- command aborted."));
 
-  make_cleanup (do_restore_user_call_depth, &user_call_depth);
+  old_chain = make_cleanup (do_restore_user_call_depth, &user_call_depth);
 
   /* Set the instream to 0, indicating execution of a
      user-defined function.  */
@@ -668,62 +711,32 @@ if_command (char *arg, int from_tty)
   free_command_lines (&command);
 }
 
-/* Cleanup */
-static void
-arg_cleanup (void *ignore)
-{
-  struct user_args *oargs = user_args;
-
-  if (!user_args)
-    internal_error (__FILE__, __LINE__,
-		    _("arg_cleanup called with no user args.\n"));
-
-  user_args = user_args->next;
-  xfree (oargs->command);
-  xfree (oargs);
-}
+/* Bind the incoming arguments for a user defined command to $arg0,
+   $arg1 ... $argN.  */
 
-/* Bind the incomming arguments for a user defined command to
-   $arg0, $arg1 ... $argMAXUSERARGS.  */
-
-static struct cleanup *
-setup_user_args (char *p)
+user_args::user_args (const char *command_line)
 {
-  struct user_args *args;
-  struct cleanup *old_chain;
-  unsigned int arg_count = 0;
-
-  args = XNEW (struct user_args);
-  memset (args, 0, sizeof (struct user_args));
-
-  args->next = user_args;
-  user_args = args;
-
-  old_chain = make_cleanup (arg_cleanup, 0/*ignored*/);
+  const char *p;
 
-  if (p == NULL)
-    return old_chain;
+  if (command_line == NULL)
+    return;
 
-  user_args->command = p = xstrdup (p);
+  m_command_line = command_line;
+  p = m_command_line.c_str ();
 
   while (*p)
     {
-      char *start_arg;
+      const char *start_arg;
       int squote = 0;
       int dquote = 0;
       int bsquote = 0;
 
-      if (arg_count >= MAXUSERARGS)
-	error (_("user defined function may only have %d arguments."),
-	       MAXUSERARGS);
-
       /* Strip whitespace.  */
       while (*p == ' ' || *p == '\t')
 	p++;
 
       /* P now points to an argument.  */
       start_arg = p;
-      user_args->a[arg_count].arg = p;
 
       /* Get to the end of this argument.  */
       while (*p)
@@ -757,11 +770,8 @@ setup_user_args (char *p)
 	    }
 	}
 
-      user_args->a[arg_count].len = p - start_arg;
-      arg_count++;
-      user_args->count++;
+      m_args.emplace_back (start_arg, p - start_arg);
     }
-  return old_chain;
 }
 
 /* Given character string P, return a point to the first argument
@@ -787,40 +797,51 @@ insert_user_defined_cmd_args (const char *line)
 {
   /* If we are not in a user-defined command, treat $argc, $arg0, et
      cetera as normal convenience variables.  */
-  if (user_args == NULL)
+  if (user_args_stack.empty ())
     return line;
 
+  std::unique_ptr<user_args> &args = user_args_stack.back ();
+  return args->insert_args (line);
+}
+
+/* Insert the user defined arguments stored in user_args into the $arg
+   arguments found in line.  */
+
+std::string
+user_args::insert_args (const char *line)
+{
   std::string new_line;
   const char *p;
 
   while ((p = locate_arg (line)))
     {
-      int i, len;
-
       new_line.append (line, p - line);
 
       if (p[4] == 'c')
 	{
-	  gdb_assert (user_args->count >= 0 && user_args->count <= 10);
-	  if (user_args->count == 10)
-	    {
-	      new_line += '1';
-	      new_line += '0';
-	    }
-	  else
-	    new_line += user_args->count + '0';
+	  new_line += std::to_string (m_args.size ());
+	  line = p + 5;
 	}
       else
 	{
-	  i = p[4] - '0';
-	  if (i >= user_args->count)
-	    error (_("Missing argument %d in user function."), i);
+	  char *tmp;
+	  unsigned long i;
 
-	  len = user_args->a[i].len;
-	  if (len > 0)
-	    new_line.append (user_args->a[i].arg, len);
+	  errno = 0;
+	  i = strtoul (p + 4, &tmp, 10);
+	  if ((i == 0 && tmp == p + 4) || errno != 0)
+	    {
+	      line = p + 4;
+	    }
+	  else if (i >= m_args.size ())
+	    error (_("Missing argument %ld in user function."), i);
+	  else
+	    {
+	      new_line.append (m_args[i].str, m_args[i].len);
+
+	      line = tmp;
+	    }
 	}
-      line = p + 5;
     }
   /* Don't forget the tail.  */
   new_line.append (line);
diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index a8f777f..3aae936 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -428,6 +428,49 @@ proc_with_prefix user_defined_command_args_stack_test {} {
     gdb_test "args_stack_command 31 32 33" $expected "execute command"
 }
 
+# Test a simple user defined command with many arguments.  GDB <= 7.12
+# used to have a hard coded limit of 10 arguments.
+
+proc_with_prefix user_defined_command_manyargs_test {} {
+    global gdb_prompt
+
+    set test "define command"
+    gdb_test_multiple "define manyargs" $test {
+	-re "End with"  {
+	    pass $test
+	}
+    }
+
+    # Define a function that doubles its arguments.
+    gdb_test \
+	[multi_line \
+	     {printf "nargs=%d:", $argc} \
+	     {set $i = 0} \
+	     {while $i < $argc} \
+	     {  eval "printf \" %%d\", 2 * $arg%d\n", $i} \
+	     {  set $i = $i + 1} \
+	     {end} \
+	     {printf "\n"} \
+	     {end}] \
+	"" \
+	"enter commands"
+
+    # Some random number of arguments, as long as higher than 10.
+    set nargs 100
+
+    set cmd "manyargs"
+    for {set i 1} {$i <= $nargs} {incr i} {
+	append cmd " $i"
+    }
+
+    set expected "nargs=$nargs:"
+    for {set i 1} {$i <= $nargs} {incr i} {
+	append expected " " [expr 2 * $i]
+    }
+
+    gdb_test $cmd $expected "execute command"
+}
+
 proc_with_prefix watchpoint_command_test {} {
     global gdb_prompt
 
@@ -950,6 +993,7 @@ breakpoint_command_test
 user_defined_command_test
 user_defined_command_args_eval
 user_defined_command_args_stack_test
+user_defined_command_manyargs_test
 watchpoint_command_test
 test_command_prompt_position
 deprecated_command_test
-- 
2.5.5

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 4/5] Test user-defined gdb commands and arguments stack
  2016-11-09  0:19 [PATCH v2 0/5] Support an "unlimited" number of user-defined arguments Pedro Alves
                   ` (3 preceding siblings ...)
  2016-11-09  0:25 ` [PATCH v2 5/5] Support an "unlimited" number of user-defined arguments Pedro Alves
@ 2016-11-09  0:26 ` Pedro Alves
  4 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2016-11-09  0:26 UTC (permalink / raw)
  To: gdb-patches

We're missing a test that makes sure that arguments to user-defined
commands are handled correctly when a user-defined command calls
another user-defined command / recurses.

The following patch changes that code, so add such a test first so we
can be confident won't be breaking this use case.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.base/commands.exp (user_defined_command_args_stack_test):
	New procedure.
	(top level): Call it.
---
 gdb/testsuite/gdb.base/commands.exp | 59 +++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index 0ac2267..a8f777f 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -370,6 +370,64 @@ proc_with_prefix user_defined_command_args_eval {} {
     gdb_test "command_args_eval 1 2 3" "argc = 3, 1 2 3" "execute command"
 }
 
+# Test that the $argc/$argN variables are pushed on/popped from the
+# args stack correctly when a user-defined command calls another
+# user-defined command (or in this case, recurses).
+
+proc_with_prefix user_defined_command_args_stack_test {} {
+    global gdb_prompt
+
+    gdb_test_multiple "define args_stack_command" \
+	"define args_stack_command" {
+	    -re "End with"  {
+		pass "define"
+	    }
+	}
+
+    # Make a command that refers to $argc/$argN before and after
+    # recursing.  Also, vary the number of arguments passed to each
+    # recursion point.
+    gdb_test \
+	[multi_line \
+	     {printf "before, argc = %d,", $argc} \
+	     {set $i = 0} \
+	     {while $i < $argc} \
+	     {  eval "printf \" %%d\", $arg%d", $i} \
+	     {  set $i = $i + 1} \
+	     {end} \
+	     {printf "\n"} \
+	     {} \
+	     {} \
+	     {if $argc == 3} \
+	     {  args_stack_command 21 22} \
+	     {end} \
+	     {if $argc == 2} \
+	     {  args_stack_command 11} \
+	     {end} \
+	     {} \
+	     {} \
+	     {printf "after, argc = %d,", $argc} \
+	     {set $i = 0} \
+	     {while $i < $argc} \
+	     {  eval "printf \" %%d\", $arg%d", $i} \
+	     {  set $i = $i + 1} \
+	     {end} \
+	     {printf "\n"} \
+	     {end}] \
+	"" \
+	"enter commands"
+
+    set expected \
+	[multi_line \
+	     "before, argc = 3, 31 32 33" \
+	     "before, argc = 2, 21 22" \
+	     "before, argc = 1, 11" \
+	     "after, argc = 1, 11" \
+	     "after, argc = 2, 21 22" \
+	     "after, argc = 3, 31 32 33"]
+    gdb_test "args_stack_command 31 32 33" $expected "execute command"
+}
+
 proc_with_prefix watchpoint_command_test {} {
     global gdb_prompt
 
@@ -891,6 +949,7 @@ infrun_breakpoint_command_test
 breakpoint_command_test
 user_defined_command_test
 user_defined_command_args_eval
+user_defined_command_args_stack_test
 watchpoint_command_test
 test_command_prompt_position
 deprecated_command_test
-- 
2.5.5

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/5] gdb/testsuite: Introduce "proc_with_prefix"
  2016-11-09  0:19 ` [PATCH v2 1/5] gdb/testsuite: Introduce "proc_with_prefix" Pedro Alves
@ 2016-11-09  1:56   ` Simon Marchi
  2016-11-09  3:10   ` [PATCH] Make gdb.mi/user-selected-context-sync.exp use proc_with_prefix Simon Marchi
  2016-11-09 11:50   ` [PATCH v2 1/5] gdb/testsuite: Introduce "proc_with_prefix" Yao Qi
  2 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2016-11-09  1:56 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On 2016-11-08 19:19, Pedro Alves wrote:
> While adding new tests to gdb.base/commands.exp, I noticed that the
> file includes a bunch of individual testcases split into their own
> procedures, and that none have ever been adjusted to use
> with_test_prefix.  Instead, each gdb_test/gdb_test_multiple/etc
> invocation takes care of including the procedure name in the test
> message, in order to make sure test messages are unique.
> 
> Simon convinced me that using the procedure name as prefix is not that
> bad of an idea:
>   https://sourceware.org/ml/gdb-patches/2016-10/msg00020.html
> 
> This commit adds an IMO simpler alternative to
> with_test_prefix_procname added by that patch -- a new
> "proc_with_prefix" convenience proc that is meant to be used in place
> of "proc", and then uses it in commands.exp.  Procedures defined with
> this automatically run their bodies under with_test_prefix $proc_name.
> 
> Here's a sample of the resulting gdb.sum diff:
> 
>  [...]
>  -PASS: gdb.base/commands.exp: break factorial #3
>  -PASS: gdb.base/commands.exp: set value to 5 in 
> test_command_prompt_position
>  -PASS: gdb.base/commands.exp: if test in test_command_prompt_position
>  -PASS: gdb.base/commands.exp: > OK in test_command_prompt_position
>  +PASS: gdb.base/commands.exp: test_command_prompt_position: break 
> factorial
>  +PASS: gdb.base/commands.exp: test_command_prompt_position: set value 
> to 5
>  +PASS: gdb.base/commands.exp: test_command_prompt_position: if test
>  +PASS: gdb.base/commands.exp: test_command_prompt_position: > OK
>  [...]

LGTM.  I think it's acceptable to use the proc name as a prefix in this 
case.  If you take care of choosing a good proc name, what you would 
pass manually to with_test_prefix otherwise would be basically the proc 
name without underscores ("test_command_prompt_position" vs "test 
command prompt position").  It can save some typing and indentation.

I'll send a patch that makes gdb.mi/user-selected-context-sync.exp use 
this new feature.

Thanks!

Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/5] Further cleanup/modernization of  gdb.base/commands.exp
  2016-11-09  0:19 ` [PATCH v2 2/5] Further cleanup/modernization of gdb.base/commands.exp Pedro Alves
@ 2016-11-09  3:02   ` Simon Marchi
  2016-11-09 15:56     ` Pedro Alves
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2016-11-09  3:02 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On 2016-11-08 19:19, Pedro Alves wrote:
> - Use multi_line for matching multi-line GDB output.
> 
>  - Add a multi_line_input variant of multi_line to build GDB input and
>    use it throughout.
> 
>    (The two changes above make the tests much more readable, IMO.)
> 
>  - Remove gdb_stop_suppressing_tests uses.
> 
>  - tighten a few regexps.
> 
>  - Replace send_gdb/gdb_expect with gdb_test_multiple and simplify,
>    making pass/fail messages the same.

I agree, the test is now much more readable.

> +    gdb_test \
> +	[multi_line_input \
> +	     "while \$foo > 0" \
> +	     "  p/x 0xfeedface" \
> +	     "  set \$foo -= 1" \
> +	     "end"] \
> +	[multi_line \
> +	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
> +	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
> +	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
> +	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
> +	     "\\\$\[0-9\]* = 0xfeedface"] \
> +	"#1"

In these instance, what is the [^\n] meant to match?  Is it the \r?  In 
that case, multi_line matches it, so I think we can get rid of them 
(throughout the file).

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] Make gdb.mi/user-selected-context-sync.exp use proc_with_prefix
  2016-11-09  0:19 ` [PATCH v2 1/5] gdb/testsuite: Introduce "proc_with_prefix" Pedro Alves
  2016-11-09  1:56   ` Simon Marchi
@ 2016-11-09  3:10   ` Simon Marchi
  2016-11-09 15:15     ` Pedro Alves
  2016-11-09 11:50   ` [PATCH v2 1/5] gdb/testsuite: Introduce "proc_with_prefix" Yao Qi
  2 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2016-11-09  3:10 UTC (permalink / raw)
  To: gdb-patches; +Cc: Pedro Alves, Simon Marchi

Pedro's patch provides a cleaner way to prefix tests with the proc name,
so let's use that.

gdb/testsuite/ChangeLog:

	* gdb.mi/user-selected-context-sync.exp (with_test_prefix_procname):
	Remove.
	(test_setup); Define with proc_with_prefix.
	(test_cli_inferior): Likewise.
	(test_cli_thread): Likewise.
	(test_cli_frame): Likewise.
	(test_cli_select_frame): Likewise.
	(test_cli_up_down): Likewise.
	(test_mi_thread_select): Likewise.
	(test_mi_stack_select_frame): Likewise.
	(test_cli_in_mi_inferior): Likewise.
	(test_cli_in_mi_thread): Likewise.
	(test_cli_in_mi_frame): Likewise.
	(top level): Do not use with_test_prefix_procname.
---
 .../gdb.mi/user-selected-context-sync.exp          | 55 +++++++++-------------
 1 file changed, 22 insertions(+), 33 deletions(-)

diff --git a/gdb/testsuite/gdb.mi/user-selected-context-sync.exp b/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
index 382763c..d3f75d8 100644
--- a/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
+++ b/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
@@ -50,17 +50,6 @@ set main_break_line [gdb_get_line_number "main break line"]
 set thread_loop_line [gdb_get_line_number "thread loop line"]
 set thread_caller_line [gdb_get_line_number "thread caller line"]
 
-# Call PROCNAME with the given arguments, inside a with_test_prefix $procname
-# block.
-
-proc with_test_prefix_procname { procname args } {
-    with_test_prefix $procname {
-	# Note: this syntax requires TCL 8.5, if we need to support 8.4,
-	# we'll need to find an alternative.
-	$procname {*}$args
-    }
-}
-
 # Return whether we expect thread THREAD to be running in mode MODE.
 #
 # MODE can be either "all-stop" or "non-stop".
@@ -377,7 +366,7 @@ proc test_continue_to_start { mode inf } {
 #
 # MODE can be either "all-stop" or "non-stop".
 
-proc test_setup { mode } {
+proc_with_prefix test_setup { mode } {
     global srcfile
     global srcdir
     global subdir
@@ -535,7 +524,7 @@ proc match_re_or_ensure_not_output { re test } {
 
 # Test selecting an inferior from CLI.
 
-proc test_cli_inferior { mode } {
+proc_with_prefix test_cli_inferior { mode } {
     global gdb_main_spawn_id mi_spawn_id
 
     reset_selection "1.1"
@@ -567,7 +556,7 @@ proc test_cli_inferior { mode } {
 
 # Test thread selection from CLI.
 
-proc test_cli_thread { mode } {
+proc_with_prefix test_cli_thread { mode } {
     global gdb_main_spawn_id
     global mi_spawn_id
 
@@ -671,7 +660,7 @@ proc test_cli_thread { mode } {
 
 # Test frame selection from CLI.
 
-proc test_cli_frame { mode } {
+proc_with_prefix test_cli_frame { mode } {
     global gdb_main_spawn_id mi_spawn_id
 
     with_test_prefix "thread 1.2" {
@@ -756,7 +745,7 @@ proc test_cli_frame { mode } {
 
 # Test frame selection from CLI with the select-frame command.
 
-proc test_cli_select_frame { mode } {
+proc_with_prefix test_cli_select_frame { mode } {
     global gdb_main_spawn_id mi_spawn_id expect_out
 
     with_test_prefix "thread 1.2" {
@@ -818,7 +807,7 @@ proc test_cli_select_frame { mode } {
 
 # Test doing an up and then down command from CLI.
 
-proc test_cli_up_down { mode } {
+proc_with_prefix test_cli_up_down { mode } {
     global gdb_main_spawn_id mi_spawn_id
 
     reset_selection "1.2"
@@ -853,7 +842,7 @@ proc test_cli_up_down { mode } {
 
 # Test selecting a thread from MI.
 
-proc test_mi_thread_select { mode } {
+proc_with_prefix test_mi_thread_select { mode } {
     global gdb_main_spawn_id mi_spawn_id
 
     reset_selection "1.1"
@@ -948,7 +937,7 @@ proc test_mi_thread_select { mode } {
     # place to test it.
 }
 
-proc test_mi_stack_select_frame { mode } {
+proc_with_prefix test_mi_stack_select_frame { mode } {
     global gdb_main_spawn_id mi_spawn_id
 
     with_test_prefix "thread 1.2" {
@@ -1019,7 +1008,7 @@ proc make_cli_in_mi_command { cli_in_mi_mode command } {
 
 # Test selecting the inferior using a CLI command in the MI channel.
 
-proc test_cli_in_mi_inferior { mode cli_in_mi_mode } {
+proc_with_prefix test_cli_in_mi_inferior { mode cli_in_mi_mode } {
     global gdb_main_spawn_id mi_spawn_id
 
     reset_selection "1.1"
@@ -1051,7 +1040,7 @@ proc test_cli_in_mi_inferior { mode cli_in_mi_mode } {
 
 # Test selecting the thread using a CLI command in the MI channel.
 
-proc test_cli_in_mi_thread { mode cli_in_mi_mode } {
+proc_with_prefix test_cli_in_mi_thread { mode cli_in_mi_mode } {
     global gdb_main_spawn_id mi_spawn_id
 
     reset_selection "1.1"
@@ -1164,7 +1153,7 @@ proc test_cli_in_mi_thread { mode cli_in_mi_mode } {
 
 # Test selecting the frame using a CLI command in the MI channel.
 
-proc test_cli_in_mi_frame { mode cli_in_mi_mode } {
+proc_with_prefix test_cli_in_mi_frame { mode cli_in_mi_mode } {
     global gdb_main_spawn_id mi_spawn_id
 
     with_test_prefix "thread 1.2" {
@@ -1258,28 +1247,28 @@ proc test_cli_in_mi_frame { mode cli_in_mi_mode } {
 }
 
 foreach_with_prefix mode { "all-stop" "non-stop" } {
-    with_test_prefix_procname test_setup $mode
+    test_setup $mode
 
     # Test selecting inferior, thread and frame from CLI
 
-    with_test_prefix_procname test_cli_inferior $mode
-    with_test_prefix_procname test_cli_thread $mode
-    with_test_prefix_procname test_cli_frame $mode
-    with_test_prefix_procname test_cli_select_frame $mode
-    with_test_prefix_procname test_cli_up_down $mode
+    test_cli_inferior $mode
+    test_cli_thread $mode
+    test_cli_frame $mode
+    test_cli_select_frame $mode
+    test_cli_up_down $mode
 
     # Test selecting thread and frame from MI
 
-    with_test_prefix_procname test_mi_thread_select $mode
-    with_test_prefix_procname test_mi_stack_select_frame $mode
+    test_mi_thread_select $mode
+    test_mi_stack_select_frame $mode
 
     # Test some CLI commands sent through MI, both with a "direct" command,
     # such as "thread 1", and with -interpreter-exec, such as
     # '-interpreter-exec console "thread 1"'.
 
     foreach_with_prefix exec_mode {"direct" "interpreter-exec"} {
-	with_test_prefix_procname test_cli_in_mi_inferior $mode $exec_mode
-	with_test_prefix_procname test_cli_in_mi_thread $mode $exec_mode
-	with_test_prefix_procname test_cli_in_mi_frame $mode $exec_mode
+	test_cli_in_mi_inferior $mode $exec_mode
+	test_cli_in_mi_thread $mode $exec_mode
+	test_cli_in_mi_frame $mode $exec_mode
     }
 }
-- 
2.10.0

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/5] gdb/testsuite: Introduce "proc_with_prefix"
  2016-11-09  0:19 ` [PATCH v2 1/5] gdb/testsuite: Introduce "proc_with_prefix" Pedro Alves
  2016-11-09  1:56   ` Simon Marchi
  2016-11-09  3:10   ` [PATCH] Make gdb.mi/user-selected-context-sync.exp use proc_with_prefix Simon Marchi
@ 2016-11-09 11:50   ` Yao Qi
  2 siblings, 0 replies; 21+ messages in thread
From: Yao Qi @ 2016-11-09 11:50 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On Wed, Nov 9, 2016 at 12:19 AM, Pedro Alves <palves@redhat.com> wrote:

> gdb/testsuite/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>
>         * gdb.base/commands.exp (gdbvar_simple_if_test)
>         (gdbvar_simple_while_test, gdbvar_complex_if_while_test)
>         (progvar_simple_if_test, progvar_simple_while_test)
>         (progvar_complex_if_while_test, if_while_breakpoint_command_test)
>         (infrun_breakpoint_command_test, breakpoint_command_test)
>         (user_defined_command_test, watchpoint_command_test)
>         (test_command_prompt_position, deprecated_command_test)
>         (bp_deleted_in_command, temporary_breakpoint_commands)
>         (stray_arg0_test, source_file_with_indented_comment)
>         (recursive_source_test, if_commands_test)
>         (error_clears_commands_left, redefine_hook_test)
>         (redefine_backtrace_test): Use proc_with_prefix.
>         * lib/gdb.exp (proc_with_prefix): New proc.

Patch is good to me.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Make gdb.mi/user-selected-context-sync.exp use proc_with_prefix
  2016-11-09  3:10   ` [PATCH] Make gdb.mi/user-selected-context-sync.exp use proc_with_prefix Simon Marchi
@ 2016-11-09 15:15     ` Pedro Alves
  2016-11-09 15:57       ` Simon Marchi
  0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2016-11-09 15:15 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 11/09/2016 03:10 AM, Simon Marchi wrote:
> Pedro's patch provides a cleaner way to prefix tests with the proc name,

(I've pushed in my patch.)

> so let's use that.
> 
>       (test_setup); Define with proc_with_prefix.
                    ^
                    |
                    + ':'

:-)

LGTM.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/5] Further cleanup/modernization of gdb.base/commands.exp
  2016-11-09  3:02   ` Simon Marchi
@ 2016-11-09 15:56     ` Pedro Alves
  2016-11-09 15:59       ` Pedro Alves
  0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2016-11-09 15:56 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On 11/09/2016 03:02 AM, Simon Marchi wrote:
> On 2016-11-08 19:19, Pedro Alves wrote:
>> - Use multi_line for matching multi-line GDB output.
>>
>>  - Add a multi_line_input variant of multi_line to build GDB input and
>>    use it throughout.
>>
>>    (The two changes above make the tests much more readable, IMO.)
>>
>>  - Remove gdb_stop_suppressing_tests uses.
>>
>>  - tighten a few regexps.
>>
>>  - Replace send_gdb/gdb_expect with gdb_test_multiple and simplify,
>>    making pass/fail messages the same.
> 
> I agree, the test is now much more readable.
> 
>> +    gdb_test \
>> +    [multi_line_input \
>> +         "while \$foo > 0" \
>> +         "  p/x 0xfeedface" \
>> +         "  set \$foo -= 1" \
>> +         "end"] \
>> +    [multi_line \
>> +         "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
>> +         "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
>> +         "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
>> +         "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
>> +         "\\\$\[0-9\]* = 0xfeedface"] \
>> +    "#1"
> 
> In these instance, what is the [^\n] meant to match?  Is it the \r?  In
> that case, multi_line matches it, so I think we can get rid of them
> (throughout the file).
> 

Indeed.  I did that now, with the patchlet below, and the test
still passes.  Also found another place where we could use multi_line.

From 9363d1c3a008337d1f56e94d8dc51e17ebd02ab0 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 9 Nov 2016 15:36:45 +0000
Subject: [PATCH] review

---
 gdb/testsuite/gdb.base/commands.exp | 49 +++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index b6c8f9e..c4bf425 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -72,10 +72,10 @@ proc_with_prefix gdbvar_simple_while_test {} {
 	     "  set \$foo -= 1" \
 	     "end"] \
 	[multi_line \
-	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
-	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
-	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
-	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface" \
+	     "\\\$\[0-9\]* = 0xfeedface" \
+	     "\\\$\[0-9\]* = 0xfeedface" \
+	     "\\\$\[0-9\]* = 0xfeedface" \
 	     "\\\$\[0-9\]* = 0xfeedface"] \
 	"#1"
 }
@@ -96,9 +96,9 @@ proc_with_prefix gdbvar_complex_if_while_test {} {
 	     "  end" \
 	     "end"] \
 	[multi_line \
-	     "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*" \
-	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
-	     "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xdeadbeef" \
+	     "\\\$\[0-9\]* = 0xfeedface" \
+	     "\\\$\[0-9\]* = 0xdeadbeef" \
 	     "\\\$\[0-9\]* = 0xfeedface"] \
 	"#1"
 }
@@ -150,10 +150,10 @@ proc_with_prefix progvar_simple_while_test {} {
 	     "  set value -= 1" \
 	     "end"] \
 	[multi_line \
-	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
-	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
-	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
-	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xfeedface" \
+	     "\\\$\[0-9\]* = 0xfeedface" \
+	     "\\\$\[0-9\]* = 0xfeedface" \
+	     "\\\$\[0-9\]* = 0xfeedface" \
 	     "\\\$\[0-9\]* = 0xfeedface"] \
 	"#1"
 }
@@ -179,9 +179,9 @@ proc_with_prefix progvar_complex_if_while_test {} {
 	     "  end" \
 	     "end"] \
 	[multi_line \
-	     "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*" \
-	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
-	     "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xdeadbeef" \
+	     "\\\$\[0-9\]* = 0xfeedface" \
+	     "\\\$\[0-9\]* = 0xdeadbeef" \
 	     "\\\$\[0-9\]* = 0xfeedface"] \
 	"#1"
 }
@@ -218,9 +218,9 @@ proc_with_prefix if_while_breakpoint_command_test {} {
     gdb_test \
 	"continue" \
 	[multi_line \
-	     "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*" \
-	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
-	     "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xdeadbeef" \
+	     "\\\$\[0-9\]* = 0xfeedface" \
+	     "\\\$\[0-9\]* = 0xdeadbeef" \
 	     "\\\$\[0-9\]* = 0xfeedface"] \
 	"#1"
     gdb_test "info break" "while.*set.*if.*p/x.*else.*p/x.*end.*"
@@ -309,9 +309,9 @@ proc_with_prefix user_defined_command_test {} {
     gdb_test \
 	"mycommand \$foo" \
 	[multi_line \
-	     "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*" \
-	     "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
-	     "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*" \
+	     "\\\$\[0-9\]* = 0xdeadbeef" \
+	     "\\\$\[0-9\]* = 0xfeedface" \
+	     "\\\$\[0-9\]* = 0xdeadbeef" \
 	     "\\\$\[0-9\]* = 0xfeedface"] \
 	"execute user-defined command"
    gdb_test "show user mycommand" \
@@ -784,7 +784,14 @@ proc_with_prefix error_clears_commands_left {} {
     gdb_test_no_output "end" "main commands 2c"
 
     gdb_run_cmd
-    gdb_test "" "hook-stop1\r\n.*\r\ncmd1\r\nUndefined command: \"errorcommandxy\"\\.  Try \"help\"\\." "cmd1 error"
+    gdb_test \
+	"" \
+	[multi_line \
+	     "hook-stop1" \
+	     ".*" \
+	     "cmd1" \
+	     "Undefined command: \"errorcommandxy\"\\.  Try \"help\"\\."] \
+	"cmd1 error"
 
     gdb_test {echo idle\n} "\r\nidle" "no cmd2"
 }
-- 
2.5.5


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Make gdb.mi/user-selected-context-sync.exp use  proc_with_prefix
  2016-11-09 15:15     ` Pedro Alves
@ 2016-11-09 15:57       ` Simon Marchi
  0 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2016-11-09 15:57 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On 2016-11-09 10:15, Pedro Alves wrote:
> On 11/09/2016 03:10 AM, Simon Marchi wrote:
>> Pedro's patch provides a cleaner way to prefix tests with the proc 
>> name,
> 
> (I've pushed in my patch.)
> 
>> so let's use that.
>> 
>>       (test_setup); Define with proc_with_prefix.
>                     ^
>                     |
>                     + ':'
> 
> :-)
> 
> LGTM.

Thanks, pushed with that fixed!

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/5] Further cleanup/modernization of gdb.base/commands.exp
  2016-11-09 15:56     ` Pedro Alves
@ 2016-11-09 15:59       ` Pedro Alves
  2016-11-09 16:09         ` Simon Marchi
  0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2016-11-09 15:59 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On 11/09/2016 03:56 PM, Pedro Alves wrote:
> On 11/09/2016 03:02 AM, Simon Marchi wrote:
>> On 2016-11-08 19:19, Pedro Alves wrote:
>>> - Use multi_line for matching multi-line GDB output.
>>>
>>>  - Add a multi_line_input variant of multi_line to build GDB input and
>>>    use it throughout.
>>>
>>>    (The two changes above make the tests much more readable, IMO.)
>>>
>>>  - Remove gdb_stop_suppressing_tests uses.
>>>
>>>  - tighten a few regexps.
>>>
>>>  - Replace send_gdb/gdb_expect with gdb_test_multiple and simplify,
>>>    making pass/fail messages the same.
>>
>> I agree, the test is now much more readable.
>>
>>> +    gdb_test \
>>> +    [multi_line_input \
>>> +         "while \$foo > 0" \
>>> +         "  p/x 0xfeedface" \
>>> +         "  set \$foo -= 1" \
>>> +         "end"] \
>>> +    [multi_line \
>>> +         "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
>>> +         "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
>>> +         "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
>>> +         "\\\$\[0-9\]* = 0xfeedface\[^\n\]*" \
>>> +         "\\\$\[0-9\]* = 0xfeedface"] \
>>> +    "#1"
>>
>> In these instance, what is the [^\n] meant to match?  Is it the \r?  In
>> that case, multi_line matches it, so I think we can get rid of them
>> (throughout the file).
>>
> 
> Indeed.  I did that now, with the patchlet below, and the test
> still passes.  Also found another place where we could use multi_line.

And I'm wondering whether this below as well would be a good idea,
or whether it'd obfuscate?  If a good idea, maybe we'd put the new
variable in gdb.exp.

From 6ad38c7fd5a2f3c2937b5de4e94d87b86894873e Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 9 Nov 2016 15:45:53 +0000
Subject: [PATCH] valnum

---
 gdb/testsuite/gdb.base/commands.exp | 89 ++++++++++++++++++++++---------------
 1 file changed, 54 insertions(+), 35 deletions(-)

diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index c4bf425..f7909dc 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -33,8 +33,13 @@ proc runto_or_return {function} {
     }
 }
 
+# A regular expression that matches a value history number.
+# E.g., $1, $2, etc.
+set valnum_re "\\\$$decimal"
+
 proc_with_prefix gdbvar_simple_if_test {} {
     global gdb_prompt
+    global valnum_re
 
     gdb_test_no_output "set \$foo = 0" "set foo"
     # All this test should do is print 0xdeadbeef once.
@@ -45,7 +50,7 @@ proc_with_prefix gdbvar_simple_if_test {} {
 	     "else" \
 	     "  p/x 0xdeadbeef" \
 	     "end"] \
-	"\\\$\[0-9\]* = 0xdeadbeef" \
+	"$valnum_re = 0xdeadbeef" \
 	"#1"
 
     # All this test should do is print 0xfeedface once.
@@ -56,12 +61,13 @@ proc_with_prefix gdbvar_simple_if_test {} {
 	     "else" \
 	     "  p/x 0xdeadbeef" \
 	     "end"] \
-	"\\\$\[0-9\]* = 0xfeedface" \
+	"$valnum_re = 0xfeedface" \
 	"#2"
 }
 
 proc_with_prefix gdbvar_simple_while_test {} {
     global gdb_prompt
+    global valnum_re
 
     gdb_test_no_output "set \$foo = 5" "set foo"
     # This test should print 0xfeedface five times.
@@ -72,16 +78,17 @@ proc_with_prefix gdbvar_simple_while_test {} {
 	     "  set \$foo -= 1" \
 	     "end"] \
 	[multi_line \
-	     "\\\$\[0-9\]* = 0xfeedface" \
-	     "\\\$\[0-9\]* = 0xfeedface" \
-	     "\\\$\[0-9\]* = 0xfeedface" \
-	     "\\\$\[0-9\]* = 0xfeedface" \
-	     "\\\$\[0-9\]* = 0xfeedface"] \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface"] \
 	"#1"
 }
 
 proc_with_prefix gdbvar_complex_if_while_test {} {
     global gdb_prompt
+    global valnum_re
 
     gdb_test_no_output "set \$foo = 4" "set foo"
     # This test should alternate between 0xdeadbeef and 0xfeedface two times.
@@ -96,15 +103,16 @@ proc_with_prefix gdbvar_complex_if_while_test {} {
 	     "  end" \
 	     "end"] \
 	[multi_line \
-	     "\\\$\[0-9\]* = 0xdeadbeef" \
-	     "\\\$\[0-9\]* = 0xfeedface" \
-	     "\\\$\[0-9\]* = 0xdeadbeef" \
-	     "\\\$\[0-9\]* = 0xfeedface"] \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface"] \
 	"#1"
 }
 
 proc_with_prefix progvar_simple_if_test {} {
     global gdb_prompt
+    global valnum_re
 
     runto_or_return factorial
 
@@ -119,7 +127,7 @@ proc_with_prefix progvar_simple_if_test {} {
 	     "else" \
 	     "  p/x 0xdeadbeef" \
 	     "end"] \
-	"\\\$\[0-9\]* = 0xdeadbeef" \
+	"$valnum_re = 0xdeadbeef" \
 	"#1"
 
     # All this test should do is print 0xfeedface once.
@@ -130,12 +138,13 @@ proc_with_prefix progvar_simple_if_test {} {
 	     "else" \
 	     "  p/x 0xdeadbeef" \
 	     "end"] \
-	"\\\$\[0-9\]* = 0xfeedface" \
+	"$valnum_re = 0xfeedface" \
 	"#2"
 }
 
 proc_with_prefix progvar_simple_while_test {} {
     global gdb_prompt
+    global valnum_re
 
     runto_or_return factorial
 
@@ -150,16 +159,17 @@ proc_with_prefix progvar_simple_while_test {} {
 	     "  set value -= 1" \
 	     "end"] \
 	[multi_line \
-	     "\\\$\[0-9\]* = 0xfeedface" \
-	     "\\\$\[0-9\]* = 0xfeedface" \
-	     "\\\$\[0-9\]* = 0xfeedface" \
-	     "\\\$\[0-9\]* = 0xfeedface" \
-	     "\\\$\[0-9\]* = 0xfeedface"] \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface"] \
 	"#1"
 }
 
 proc_with_prefix progvar_complex_if_while_test {} {
     global gdb_prompt
+    global valnum_re
 
     runto_or_return factorial
 
@@ -179,14 +189,16 @@ proc_with_prefix progvar_complex_if_while_test {} {
 	     "  end" \
 	     "end"] \
 	[multi_line \
-	     "\\\$\[0-9\]* = 0xdeadbeef" \
-	     "\\\$\[0-9\]* = 0xfeedface" \
-	     "\\\$\[0-9\]* = 0xdeadbeef" \
-	     "\\\$\[0-9\]* = 0xfeedface"] \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface"] \
 	"#1"
 }
 
 proc_with_prefix if_while_breakpoint_command_test {} {
+    global valnum_re
+
     runto_or_return factorial
 
     # Don't depend upon argument passing, since most simulators don't
@@ -218,10 +230,10 @@ proc_with_prefix if_while_breakpoint_command_test {} {
     gdb_test \
 	"continue" \
 	[multi_line \
-	     "\\\$\[0-9\]* = 0xdeadbeef" \
-	     "\\\$\[0-9\]* = 0xfeedface" \
-	     "\\\$\[0-9\]* = 0xdeadbeef" \
-	     "\\\$\[0-9\]* = 0xfeedface"] \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface"] \
 	"#1"
     gdb_test "info break" "while.*set.*if.*p/x.*else.*p/x.*end.*"
 }
@@ -282,6 +294,7 @@ proc_with_prefix breakpoint_command_test {} {
 # Test a simple user defined command (with arguments)
 proc_with_prefix user_defined_command_test {} {
     global gdb_prompt
+    global valnum_re
 
     gdb_test_no_output "set \$foo = 4" "set foo"
 
@@ -306,13 +319,16 @@ proc_with_prefix user_defined_command_test {} {
 	"" \
 	"enter commands"
 
+    global decimal
+    set valnum_re "\\\$$decimal"
+
     gdb_test \
 	"mycommand \$foo" \
 	[multi_line \
-	     "\\\$\[0-9\]* = 0xdeadbeef" \
-	     "\\\$\[0-9\]* = 0xfeedface" \
-	     "\\\$\[0-9\]* = 0xdeadbeef" \
-	     "\\\$\[0-9\]* = 0xfeedface"] \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface"] \
 	"execute user-defined command"
    gdb_test "show user mycommand" \
 	"  while \\\$arg0.*set.*    if \\\(\\\$arg0.*p/x.*    else\[^\n\].*p/x.*    end\[^\n\].*  end\[^\n\].*" \
@@ -414,6 +430,7 @@ proc_with_prefix watchpoint_command_test {} {
 
 proc_with_prefix test_command_prompt_position {} {
     global gdb_prompt
+    global valnum_re
 
     runto_or_return factorial
 
@@ -430,7 +447,7 @@ proc_with_prefix test_command_prompt_position {} {
 	     "else" \
 	     "  p/x 0xdeadbeef" \
 	     "end"] \
-	"\\\$\[0-9\]* = 0xdeadbeef" \
+	"$valnum_re = 0xdeadbeef" \
 	"if test"
 
     # Now let's test for the correct position of the '>' in gdb's
@@ -579,20 +596,22 @@ proc_with_prefix temporary_breakpoint_commands {} {
 # Test that GDB can handle $arg0 outside of user functions without
 # crashing.
 proc_with_prefix stray_arg0_test { } {
+    global valnum_re
+
     gdb_test "print \$arg0" \
-	"\\\$\[0-9\]* = void" \
+	"$valnum_re = void" \
 	"#1"
 
     gdb_test "if 1 == 1\nprint \$arg0\nend" \
-	"\\\$\[0-9\]* = void" \
+	"$valnum_re = void" \
 	"#2"
 
     gdb_test "print \$arg0 = 1" \
-	"\\\$\[0-9\]* = 1" \
+	"$valnum_re = 1" \
 	"#3"
 
     gdb_test "print \$arg0" \
-	"\\\$\[0-9\]* = 1" \
+	"$valnum_re = 1" \
 	"#4"
 }
 
-- 
2.5.5


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/5] Further cleanup/modernization of  gdb.base/commands.exp
  2016-11-09 15:59       ` Pedro Alves
@ 2016-11-09 16:09         ` Simon Marchi
  2016-11-09 16:16           ` Pedro Alves
  2016-11-09 16:22           ` Pedro Alves
  0 siblings, 2 replies; 21+ messages in thread
From: Simon Marchi @ 2016-11-09 16:09 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On 2016-11-09 10:59, Pedro Alves wrote:
> And I'm wondering whether this below as well would be a good idea,
> or whether it'd obfuscate?  If a good idea, maybe we'd put the new
> variable in gdb.exp.

...

> @@ -45,7 +50,7 @@ proc_with_prefix gdbvar_simple_if_test {} {
>  	     "else" \
>  	     "  p/x 0xdeadbeef" \
>  	     "end"] \
> -	"\\\$\[0-9\]* = 0xdeadbeef" \
> +	"$valnum_re = 0xdeadbeef" \
>  	"#1"


That's not bad.  I was going to suggest using {} instead of "" to get 
rid of most backslashes (untested):

-"\\\$\[0-9\]* = 0xdeadbeef"
+{\$[0-9]* = 0xdeadbeef}

but with the variable it looks good as well.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/5] Further cleanup/modernization of gdb.base/commands.exp
  2016-11-09 16:09         ` Simon Marchi
@ 2016-11-09 16:16           ` Pedro Alves
  2016-11-09 16:25             ` Simon Marchi
  2016-11-09 16:22           ` Pedro Alves
  1 sibling, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2016-11-09 16:16 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On 11/09/2016 04:09 PM, Simon Marchi wrote:
> On 2016-11-09 10:59, Pedro Alves wrote:
>> And I'm wondering whether this below as well would be a good idea,
>> or whether it'd obfuscate?  If a good idea, maybe we'd put the new
>> variable in gdb.exp.
> 
> ...
> 
>> @@ -45,7 +50,7 @@ proc_with_prefix gdbvar_simple_if_test {} {
>>           "else" \
>>           "  p/x 0xdeadbeef" \
>>           "end"] \
>> -    "\\\$\[0-9\]* = 0xdeadbeef" \
>> +    "$valnum_re = 0xdeadbeef" \
>>      "#1"
> 
> 
> That's not bad.  I was going to suggest using {} instead of "" to get
> rid of most backslashes (untested):
> 
> -"\\\$\[0-9\]* = 0xdeadbeef"
> +{\$[0-9]* = 0xdeadbeef}
> 
> but with the variable it looks good as well.
> 

Here's the resulting squashed patch then.  WDYT?

From 55c622b20b7b4ddba7dd01ac30db182c5fb6ef18 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 9 Nov 2016 16:10:44 +0000
Subject: [PATCH] Further cleanup/modernization of gdb.base/commands.exp

 - Use multi_line for matching multi-line GDB output.

 - Add a multi_line_input variant of multi_line to build GDB input and
   use it throughout.

   (The two changes above make the tests much more readable, IMO.)

 - Add a new valnum_re global to get rid of the multiple "\\\$\[0-9\]*".

 - Remove gdb_stop_suppressing_tests uses.

 - tighten a few regexps.

 - Replace send_gdb/gdb_expect with gdb_test_multiple and simplify,
   making pass/fail messages the same.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.base/commands.exp (runto_or_return): New
	procedure.
	(gdbvar_simple_if_test, gdbvar_simple_while_test)
	(gdbvar_complex_if_while_test, progvar_simple_if_test)
	(progvar_simple_while_test, progvar_complex_if_while_test)
	(if_while_breakpoint_command_test)
	(infrun_breakpoint_command_test, breakpoint_command_test)
	(user_defined_command_test, watchpoint_command_test)
	(test_command_prompt_position, redefine_hook_test)
	(stray_arg0_test, error_clears_commands_left, redefine_hook_test)
	(redefine_backtrace_test): Use runto_or_return, $valnum_re,
	multi_line_input and multi_line.  Remove gdb_expect and
	gdb_stop_suppressing_tests uses.
	* lib/gdb.exp (valnum_re): New global.
	(multi_line_input): New procedure.
---
 gdb/testsuite/gdb.base/commands.exp | 365 ++++++++++++++++++++++++------------
 gdb/testsuite/lib/gdb.exp           |  13 ++
 2 files changed, 257 insertions(+), 121 deletions(-)

diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index fe2c23a..44947f2 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -23,91 +23,183 @@ if { [prepare_for_testing commands.exp commands run.c {debug additional_flags=-D
     return -1
 }
 
+# Run to FUNCTION.  If that fails, issue a FAIL and make the caller
+# return.
+
+proc runto_or_return {function} {
+    if { ![runto factorial] } {
+	fail "cannot run to $function"
+	return -code return
+    }
+}
+
 proc_with_prefix gdbvar_simple_if_test {} {
     global gdb_prompt
+    global valnum_re
 
     gdb_test_no_output "set \$foo = 0" "set foo"
     # All this test should do is print 0xdeadbeef once.
-    gdb_test "if \$foo == 1\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
-	    "\\\$\[0-9\]* = 0xdeadbeef" "#1"
+    gdb_test \
+	[multi_line_input \
+	     "if \$foo == 1" \
+	     "  p/x 0xfeedface" \
+	     "else" \
+	     "  p/x 0xdeadbeef" \
+	     "end"] \
+	"$valnum_re = 0xdeadbeef" \
+	"#1"
+
     # All this test should do is print 0xfeedface once.
-    gdb_test "if \$foo == 0\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
-	    "\\\$\[0-9\]* = 0xfeedface" "#2"
+    gdb_test \
+	[multi_line_input \
+	     "if \$foo == 0" \
+	     "  p/x 0xfeedface" \
+	     "else" \
+	     "  p/x 0xdeadbeef" \
+	     "end"] \
+	"$valnum_re = 0xfeedface" \
+	"#2"
 }
 
 proc_with_prefix gdbvar_simple_while_test {} {
     global gdb_prompt
+    global valnum_re
 
     gdb_test_no_output "set \$foo = 5" "set foo"
     # This test should print 0xfeedface five times.
-    gdb_test "while \$foo > 0\np/x 0xfeedface\nset \$foo -= 1\nend" \
-	    "\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "#1"
+    gdb_test \
+	[multi_line_input \
+	     "while \$foo > 0" \
+	     "  p/x 0xfeedface" \
+	     "  set \$foo -= 1" \
+	     "end"] \
+	[multi_line \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface"] \
+	"#1"
 }
 
 proc_with_prefix gdbvar_complex_if_while_test {} {
     global gdb_prompt
+    global valnum_re
 
-    gdb_test_no_output "set \$foo = 4" \
-	"set foo"
+    gdb_test_no_output "set \$foo = 4" "set foo"
     # This test should alternate between 0xdeadbeef and 0xfeedface two times.
-    gdb_test "while \$foo > 0\nset \$foo -= 1\nif \(\$foo % 2\) == 1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend" \
-	    "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "#1"
+    gdb_test \
+	[multi_line_input \
+	     "while \$foo > 0" \
+	     "  set \$foo -= 1" \
+	     "  if \(\$foo % 2\) == 1" \
+	     "    p/x 0xdeadbeef" \
+	     "  else" \
+	     "    p/x 0xfeedface" \
+	     "  end" \
+	     "end"] \
+	[multi_line \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface"] \
+	"#1"
 }
 
 proc_with_prefix progvar_simple_if_test {} {
     global gdb_prompt
+    global valnum_re
+
+    runto_or_return factorial
 
-    if { ![runto factorial] } then { gdb_suppress_tests; }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=5" ".*" "set value to 5"
+    gdb_test "p value=5" " = 5" "set value to 5"
     # All this test should do is print 0xdeadbeef once.
-    gdb_test "if value == 1\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
-	    "\\\$\[0-9\]* = 0xdeadbeef" \
-	    "#1"
+    gdb_test \
+	[multi_line_input \
+	     "if value == 1" \
+	     "  p/x 0xfeedface" \
+	     "else" \
+	     "  p/x 0xdeadbeef" \
+	     "end"] \
+	"$valnum_re = 0xdeadbeef" \
+	"#1"
+
     # All this test should do is print 0xfeedface once.
-    gdb_test "if value == 5\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
-	    "\\\$\[0-9\]* = 0xfeedface" \
-	    "#2"
-    gdb_stop_suppressing_tests
+    gdb_test \
+	[multi_line_input \
+	     "if value == 5" \
+	     "  p/x 0xfeedface" \
+	     "else" \
+	     "  p/x 0xdeadbeef" \
+	     "end"] \
+	"$valnum_re = 0xfeedface" \
+	"#2"
 }
 
 proc_with_prefix progvar_simple_while_test {} {
     global gdb_prompt
+    global valnum_re
+
+    runto_or_return factorial
 
-    if { ![runto factorial] } then { gdb_suppress_tests }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=5" ".*" "set value to 5"
+    gdb_test "p value=5" " = 5" "set value to 5"
     # This test should print 0xfeedface five times.
-    gdb_test "while value > 0\np/x 0xfeedface\nset value -= 1\nend" \
-	    "\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "#1"
-    gdb_stop_suppressing_tests
+    gdb_test \
+	[multi_line_input \
+	     "while value > 0" \
+	     "  p/x 0xfeedface" \
+	     "  set value -= 1" \
+	     "end"] \
+	[multi_line \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface"] \
+	"#1"
 }
 
 proc_with_prefix progvar_complex_if_while_test {} {
     global gdb_prompt
+    global valnum_re
+
+    runto_or_return factorial
 
-    if { ![runto factorial] } then { gdb_suppress_tests }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=4" ".*" "set value to 4"
-    # This test should alternate between 0xdeadbeef and 0xfeedface two times.
-    gdb_test "while value > 0\nset value -= 1\nif \(value % 2\) == 1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend" \
-	    "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "#1"
-    gdb_stop_suppressing_tests
+    gdb_test "p value=4" " = 4" "set value to 4"
+    # This test should alternate between 0xdeadbeef and 0xfeedface two
+    # times.
+    gdb_test \
+	[multi_line_input \
+	     "while value > 0" \
+	     "  set value -= 1" \
+	     "  if \(value % 2\) == 1" \
+	     "    p/x 0xdeadbeef" \
+	     "  else" \
+	     "    p/x 0xfeedface" \
+	     "  end" \
+	     "end"] \
+	[multi_line \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface"] \
+	"#1"
 }
 
 proc_with_prefix if_while_breakpoint_command_test {} {
+    global valnum_re
+
+    runto_or_return factorial
 
-    if { ![runto factorial] } then { gdb_suppress_tests }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=5" ".*" "set value to 5"
+    gdb_test "p value=5" " = 5" "set value to 5"
     delete_breakpoints
     gdb_test "break factorial" "Breakpoint.*at.*" "break factorial"
 
@@ -118,14 +210,28 @@ proc_with_prefix if_while_breakpoint_command_test {} {
     }
 
     # This test should alternate between 0xdeadbeef and 0xfeedface two times.
-    gdb_test "while value > 0\nset value -= 1\nif \(value % 2\) == 1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend\nend" \
-	    "" \
-	    "commands part 2"
-    gdb_test "continue" \
-	    "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "#1"
+    gdb_test \
+	[multi_line_input \
+	     "while value > 0" \
+	     "  set value -= 1" \
+	     "  if \(value % 2\) == 1" \
+	     "    p/x 0xdeadbeef" \
+	     "  else" \
+	     "    p/x 0xfeedface" \
+	     "  end" \
+	     "end" \
+	     "end"] \
+	"" \
+	"commands part 2"
+    gdb_test \
+	"continue" \
+	[multi_line \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface"] \
+	"#1"
     gdb_test "info break" "while.*set.*if.*p/x.*else.*p/x.*end.*"
-    gdb_stop_suppressing_tests
 }
 
 # Test that we can run the inferior from breakpoint commands.
@@ -135,11 +241,11 @@ proc_with_prefix if_while_breakpoint_command_test {} {
 # subsection "Breakpoint command lists".
 
 proc_with_prefix infrun_breakpoint_command_test {} {
+    runto_or_return factorial
 
-    if { ![runto factorial] } then { gdb_suppress_tests }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=6" ".*" "set value to 6"
+    gdb_test "p value=6" " = 6" "set value to 6"
     delete_breakpoints
     gdb_test "break factorial if value == 5" "Breakpoint.*at.*"
 
@@ -159,29 +265,32 @@ proc_with_prefix infrun_breakpoint_command_test {} {
 
     gdb_test "continue" \
 	"Continuing.*.*.*Breakpoint \[0-9\]*, factorial \\(value=5\\).*at.*\[0-9\]*\[      \]*if \\(value > 1\\) \{.*\[0-9\]*\[      \]*value \\*= factorial \\(value - 1\\);.*"
-
-    gdb_stop_suppressing_tests
 }
 
 proc_with_prefix breakpoint_command_test {} {
+    runto_or_return factorial
 
-    if { ![runto factorial] } then { gdb_suppress_tests; }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=6" ".*" "set value to 6"
+    gdb_test "p value=6" " = 6" "set value to 6"
     delete_breakpoints
     gdb_test "break factorial" "Breakpoint.*at.*"
-    gdb_test "commands\nprintf \"Now the value is %d\\n\", value\nend" \
-	"End with.*" "commands"
+    gdb_test \
+	[multi_line_input \
+	     "commands" \
+	     "  printf \"Now the value is %d\\n\", value" \
+	     "end"] \
+	"End with.*" \
+	"commands"
     gdb_test "continue" \
 	    "Breakpoint \[0-9\]*, factorial.*Now the value is 5"
     gdb_test "print value" " = 5"
-    gdb_stop_suppressing_tests
 }
 
 # Test a simple user defined command (with arguments)
 proc_with_prefix user_defined_command_test {} {
     global gdb_prompt
+    global valnum_re
 
     gdb_test_no_output "set \$foo = 4" "set foo"
 
@@ -192,13 +301,31 @@ proc_with_prefix user_defined_command_test {} {
     }
 
     # This test should alternate between 0xdeadbeef and 0xfeedface two times.
-    gdb_test "while \$arg0 > 0\nset \$arg0 -= 1\nif \(\$arg0 % 2\) == 1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend\nend" \
-	    "" \
-	    "enter commands"
-
-    gdb_test "mycommand \$foo" \
-	    "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "execute user-defined command"
+    gdb_test \
+	[multi_line_input \
+	     "while \$arg0 > 0" \
+	     "  set \$arg0 -= 1" \
+	     "  if \(\$arg0 % 2\) == 1" \
+	     "    p/x 0xdeadbeef" \
+	     "  else" \
+	     "    p/x 0xfeedface" \
+	     "  end" \
+	     "end" \
+	     "end"] \
+	"" \
+	"enter commands"
+
+    global decimal
+    set valnum_re "\\\$$decimal"
+
+    gdb_test \
+	"mycommand \$foo" \
+	[multi_line \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface"] \
+	"execute user-defined command"
    gdb_test "show user mycommand" \
 	"  while \\\$arg0.*set.*    if \\\(\\\$arg0.*p/x.*    else\[^\n\].*p/x.*    end\[^\n\].*  end\[^\n\].*" \
 	   "display user command"
@@ -233,7 +360,8 @@ proc_with_prefix watchpoint_command_test {} {
 	gdb_test_no_output "set can-use-hw-watchpoints 0" ""
     }
 
-    if { ![runto factorial] } then { return }
+    runto_or_return factorial
+
     delete_breakpoints
 
     # Verify that we can create a watchpoint, and give it a commands
@@ -298,60 +426,44 @@ proc_with_prefix watchpoint_command_test {} {
 
 proc_with_prefix test_command_prompt_position {} {
     global gdb_prompt
+    global valnum_re
+
+    runto_or_return factorial
 
-    if { ![runto factorial] } then { gdb_suppress_tests; }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
     delete_breakpoints
     gdb_test "break factorial" "Breakpoint.*at.*"
     gdb_test "p value=5" ".*" "set value to 5"
     # All this test should do is print 0xdeadbeef once.
-    gdb_test "if value == 1\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
-	    "\\\$\[0-9\]* = 0xdeadbeef" \
-	    "if test"
-    
+    gdb_test \
+	[multi_line_input \
+	     "if value == 1" \
+	     "  p/x 0xfeedface" \
+	     "else" \
+	     "  p/x 0xdeadbeef" \
+	     "end"] \
+	"$valnum_re = 0xdeadbeef" \
+	"if test"
+
     # Now let's test for the correct position of the '>' in gdb's
     # prompt for commands.  It should be at the beginning of the line,
     # and not after one space.
 
-    send_gdb "commands\n"
-    gdb_expect {
-	-re "Type commands.*End with.*\[\r\n\]>$" { 
-	    send_gdb "printf \"Now the value is %d\\n\", value\n"
-	    gdb_expect {
+    set test "> OK"
+    gdb_test_multiple "commands" $test {
+	-re "Type commands.*End with.*\[\r\n\]>$" {
+	    gdb_test_multiple "printf \"Now the value is %d\\n\", value" $test {
 		-re "^printf.*value\r\n>$" {
-		    send_gdb "end\n"
-		    gdb_expect {
+		    gdb_test_multiple "end" $test {
 			-re "^end\r\n$gdb_prompt $" { 
-			    pass "> OK"
-			}
-			-re ".*$gdb_prompt $" { 
-			    fail "some other message"
-			}
-			timeout  { 
-			    fail "(timeout) 1"
+			    pass $test
 			}
 		    }
 		}
-		-re "^ >$" { fail "> not OK" }
-		-re ".*$gdb_prompt $"   { 
-		    fail "wrong message"
-		}
-		timeout    { 
-		    fail "(timeout) 2"
-		}
 	    }
 	}
-	-re "Type commands.*End with.*\[\r\n\] >$" { 
-	    fail "prompt not OK"
-	}
-	-re ".*$gdb_prompt $" { 
-	    fail "commands"
-	}
-	timeout { fail "(timeout) 3" }
     }
-
-    gdb_stop_suppressing_tests
 }
 
 
@@ -480,20 +592,22 @@ proc_with_prefix temporary_breakpoint_commands {} {
 # Test that GDB can handle $arg0 outside of user functions without
 # crashing.
 proc_with_prefix stray_arg0_test { } {
+    global valnum_re
+
     gdb_test "print \$arg0" \
-	"\\\$\[0-9\]* = void" \
+	"$valnum_re = void" \
 	"#1"
 
     gdb_test "if 1 == 1\nprint \$arg0\nend" \
-	"\\\$\[0-9\]* = void" \
+	"$valnum_re = void" \
 	"#2"
 
     gdb_test "print \$arg0 = 1" \
-	"\\\$\[0-9\]* = 1" \
+	"$valnum_re = 1" \
 	"#3"
 
     gdb_test "print \$arg0" \
-	"\\\$\[0-9\]* = 1" \
+	"$valnum_re = 1" \
 	"#4"
 }
 
@@ -685,7 +799,14 @@ proc_with_prefix error_clears_commands_left {} {
     gdb_test_no_output "end" "main commands 2c"
 
     gdb_run_cmd
-    gdb_test "" "hook-stop1\r\n.*\r\ncmd1\r\nUndefined command: \"errorcommandxy\"\\.  Try \"help\"\\." "cmd1 error"
+    gdb_test \
+	"" \
+	[multi_line \
+	     "hook-stop1" \
+	     ".*" \
+	     "cmd1" \
+	     "Undefined command: \"errorcommandxy\"\\.  Try \"help\"\\."] \
+	"cmd1 error"
 
     gdb_test {echo idle\n} "\r\nidle" "no cmd2"
 }
@@ -693,13 +814,20 @@ proc_with_prefix error_clears_commands_left {} {
 proc_with_prefix redefine_hook_test {} {
     global gdb_prompt
 
-    gdb_test "define one\nend" \
-      "" \
-      "define one"
+    gdb_test \
+	[multi_line_input \
+	     "define one"\
+	     "end"] \
+	"" \
+	"define one"
 
-    gdb_test "define hook-one\necho hibob\\n\nend" \
-      "" \
-      "define hook-one"
+    gdb_test \
+	[multi_line_input \
+	     "define hook-one" \
+	     "echo hibob\\n" \
+	     "end"] \
+	"" \
+	"define hook-one"
 
     set test "redefine one"
     gdb_test_multiple "define one" $test {
@@ -713,13 +841,9 @@ proc_with_prefix redefine_hook_test {} {
 	}
     }
 
-    gdb_test "end" \
-	    "" \
-	    "enter commands for one redefinition"
+    gdb_test "end" "" "enter commands for one redefinition"
 
-    gdb_test "one" \
-	    "hibob" \
-	    "execute one command"
+    gdb_test "one" "hibob" "execute one command"
 }
 
 proc_with_prefix redefine_backtrace_test {} {
@@ -737,16 +861,15 @@ proc_with_prefix redefine_backtrace_test {} {
 	}
     }
 
-    gdb_test "echo hibob\\n\nend" \
-	    "" \
-	    "enter commands"
+    gdb_test \
+	[multi_line_input \
+	     "echo hibob\\n" \
+	     "end"] \
+	"" \
+	"enter commands"
 
-    gdb_test "backtrace" \
-	    "hibob" \
-	    "execute backtrace command"
-    gdb_test "bt" \
-	    "hibob" \
-	    "execute bt command"
+    gdb_test "backtrace" "hibob" "execute backtrace command"
+    gdb_test "bt" "hibob" "execute bt command"
 }
 
 gdbvar_simple_if_test
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 735ed11..e1e9880 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -117,6 +117,10 @@ set octal "\[0-7\]+"
 
 set inferior_exited_re "(\\\[Inferior \[0-9\]+ \\(.*\\) exited)"
 
+# A regular expression that matches a value history number.
+# E.g., $1, $2, etc.
+set valnum_re "\\\$$decimal"
+
 ### Only procedures should come after this point.
 
 #
@@ -6005,5 +6009,14 @@ proc multi_line { args } {
     return [join $args "\r\n"]
 }
 
+# Similar to the above, but while multi_line is meant to be used to
+# match GDB output, this one is meant to be used to build strings to
+# send as GDB input.
+
+proc multi_line_input { args } {
+    return [join $args "\n"]
+}
+
+
 # Always load compatibility stuff.
 load_lib future.exp
-- 
2.5.5


-- 
Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/5] Further cleanup/modernization of gdb.base/commands.exp
  2016-11-09 16:09         ` Simon Marchi
  2016-11-09 16:16           ` Pedro Alves
@ 2016-11-09 16:22           ` Pedro Alves
  2016-11-09 16:37             ` Pedro Alves
  1 sibling, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2016-11-09 16:22 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On 11/09/2016 04:09 PM, Simon Marchi wrote:
> On 2016-11-09 10:59, Pedro Alves wrote:
>> And I'm wondering whether this below as well would be a good idea,
>> or whether it'd obfuscate?  If a good idea, maybe we'd put the new
>> variable in gdb.exp.
> 
> ...
> 
>> @@ -45,7 +50,7 @@ proc_with_prefix gdbvar_simple_if_test {} {
>>           "else" \
>>           "  p/x 0xdeadbeef" \
>>           "end"] \
>> -    "\\\$\[0-9\]* = 0xdeadbeef" \
>> +    "$valnum_re = 0xdeadbeef" \
>>      "#1"
> 
> 
> That's not bad.  I was going to suggest using {} instead of "" to get
> rid of most backslashes (untested):
> 
> -"\\\$\[0-9\]* = 0xdeadbeef"
> +{\$[0-9]* = 0xdeadbeef}

I strikes me that I can use {}when building the input strings.  I was
already doing it in the new tests added by the following patches, but
not in this one, for some reason.  Let me give that a try.

> 
> but with the variable it looks good as well.
> 

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/5] Further cleanup/modernization of  gdb.base/commands.exp
  2016-11-09 16:16           ` Pedro Alves
@ 2016-11-09 16:25             ` Simon Marchi
  2016-11-09 18:51               ` Pedro Alves
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2016-11-09 16:25 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On 2016-11-09 11:16, Pedro Alves wrote:
> On 11/09/2016 04:09 PM, Simon Marchi wrote:
>> On 2016-11-09 10:59, Pedro Alves wrote:
>>> And I'm wondering whether this below as well would be a good idea,
>>> or whether it'd obfuscate?  If a good idea, maybe we'd put the new
>>> variable in gdb.exp.
>> 
>> ...
>> 
>>> @@ -45,7 +50,7 @@ proc_with_prefix gdbvar_simple_if_test {} {
>>>           "else" \
>>>           "  p/x 0xdeadbeef" \
>>>           "end"] \
>>> -    "\\\$\[0-9\]* = 0xdeadbeef" \
>>> +    "$valnum_re = 0xdeadbeef" \
>>>      "#1"
>> 
>> 
>> That's not bad.  I was going to suggest using {} instead of "" to get
>> rid of most backslashes (untested):
>> 
>> -"\\\$\[0-9\]* = 0xdeadbeef"
>> +{\$[0-9]* = 0xdeadbeef}
>> 
>> but with the variable it looks good as well.
>> 
> 
> Here's the resulting squashed patch then.  WDYT?

LGTM.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/5] Further cleanup/modernization of gdb.base/commands.exp
  2016-11-09 16:22           ` Pedro Alves
@ 2016-11-09 16:37             ` Pedro Alves
  2016-11-09 16:41               ` Pedro Alves
  0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2016-11-09 16:37 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On 11/09/2016 04:22 PM, Pedro Alves wrote:
> On 11/09/2016 04:09 PM, Simon Marchi wrote:

>> That's not bad.  I was going to suggest using {} instead of "" to get
>> rid of most backslashes (untested):
>>
>> -"\\\$\[0-9\]* = 0xdeadbeef"
>> +{\$[0-9]* = 0xdeadbeef}
> 
> I strikes me that I can use {}when building the input strings.  I was
> already doing it in the new tests added by the following patches, but
> not in this one, for some reason.  Let me give that a try.

Like this.  Done throughout for consistency.

Must stop polishing this.  :-)

From afc965b18044f47c15a3eae9a8d8adb3211b1716 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 9 Nov 2016 16:21:30 +0000
Subject: [PATCH] {}

---
 gdb/testsuite/gdb.base/commands.exp | 142 ++++++++++++++++++------------------
 1 file changed, 71 insertions(+), 71 deletions(-)

diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index 44947f2..ba94174 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -41,22 +41,22 @@ proc_with_prefix gdbvar_simple_if_test {} {
     # All this test should do is print 0xdeadbeef once.
     gdb_test \
 	[multi_line_input \
-	     "if \$foo == 1" \
-	     "  p/x 0xfeedface" \
-	     "else" \
-	     "  p/x 0xdeadbeef" \
-	     "end"] \
+	     {if $foo == 1} \
+	     {  p/x 0xfeedface} \
+	     {else} \
+	     {  p/x 0xdeadbeef} \
+	     {end}] \
 	"$valnum_re = 0xdeadbeef" \
 	"#1"
 
     # All this test should do is print 0xfeedface once.
     gdb_test \
 	[multi_line_input \
-	     "if \$foo == 0" \
-	     "  p/x 0xfeedface" \
-	     "else" \
-	     "  p/x 0xdeadbeef" \
-	     "end"] \
+	     {if $foo == 0} \
+	     {  p/x 0xfeedface} \
+	     {else} \
+	     {  p/x 0xdeadbeef} \
+	     {end}] \
 	"$valnum_re = 0xfeedface" \
 	"#2"
 }
@@ -69,10 +69,10 @@ proc_with_prefix gdbvar_simple_while_test {} {
     # This test should print 0xfeedface five times.
     gdb_test \
 	[multi_line_input \
-	     "while \$foo > 0" \
-	     "  p/x 0xfeedface" \
-	     "  set \$foo -= 1" \
-	     "end"] \
+	     {while $foo > 0} \
+	     {  p/x 0xfeedface} \
+	     {set $foo -= 1} \
+	     {end}] \
 	[multi_line \
 	     "$valnum_re = 0xfeedface" \
 	     "$valnum_re = 0xfeedface" \
@@ -90,14 +90,14 @@ proc_with_prefix gdbvar_complex_if_while_test {} {
     # This test should alternate between 0xdeadbeef and 0xfeedface two times.
     gdb_test \
 	[multi_line_input \
-	     "while \$foo > 0" \
-	     "  set \$foo -= 1" \
-	     "  if \(\$foo % 2\) == 1" \
-	     "    p/x 0xdeadbeef" \
-	     "  else" \
-	     "    p/x 0xfeedface" \
-	     "  end" \
-	     "end"] \
+	     {while $foo > 0} \
+	     {  set $foo -= 1} \
+	     {  if ($foo % 2) == 1} \
+	     {    p/x 0xdeadbeef} \
+	     {  else} \
+	     {    p/x 0xfeedface} \
+	     {  end} \
+	     {end}] \
 	[multi_line \
 	     "$valnum_re = 0xdeadbeef" \
 	     "$valnum_re = 0xfeedface" \
@@ -118,22 +118,22 @@ proc_with_prefix progvar_simple_if_test {} {
     # All this test should do is print 0xdeadbeef once.
     gdb_test \
 	[multi_line_input \
-	     "if value == 1" \
-	     "  p/x 0xfeedface" \
-	     "else" \
-	     "  p/x 0xdeadbeef" \
-	     "end"] \
+	     {if value == 1} \
+	     {  p/x 0xfeedface} \
+	     {else} \
+	     {  p/x 0xdeadbeef} \
+	     {end}] \
 	"$valnum_re = 0xdeadbeef" \
 	"#1"
 
     # All this test should do is print 0xfeedface once.
     gdb_test \
 	[multi_line_input \
-	     "if value == 5" \
-	     "  p/x 0xfeedface" \
-	     "else" \
-	     "  p/x 0xdeadbeef" \
-	     "end"] \
+	     {if value == 5} \
+	     {  p/x 0xfeedface} \
+	     {else} \
+	     {  p/x 0xdeadbeef} \
+	     {end}] \
 	"$valnum_re = 0xfeedface" \
 	"#2"
 }
@@ -150,10 +150,10 @@ proc_with_prefix progvar_simple_while_test {} {
     # This test should print 0xfeedface five times.
     gdb_test \
 	[multi_line_input \
-	     "while value > 0" \
-	     "  p/x 0xfeedface" \
-	     "  set value -= 1" \
-	     "end"] \
+	     {while value > 0} \
+	     {  p/x 0xfeedface} \
+	     {  set value -= 1} \
+	     {end}] \
 	[multi_line \
 	     "$valnum_re = 0xfeedface" \
 	     "$valnum_re = 0xfeedface" \
@@ -176,14 +176,14 @@ proc_with_prefix progvar_complex_if_while_test {} {
     # times.
     gdb_test \
 	[multi_line_input \
-	     "while value > 0" \
-	     "  set value -= 1" \
-	     "  if \(value % 2\) == 1" \
-	     "    p/x 0xdeadbeef" \
-	     "  else" \
-	     "    p/x 0xfeedface" \
-	     "  end" \
-	     "end"] \
+	     {while value > 0} \
+	     {  set value -= 1} \
+	     {  if (value % 2) == 1} \
+	     {    p/x 0xdeadbeef} \
+	     {  else} \
+	     {    p/x 0xfeedface} \
+	     {  end} \
+	     {end}] \
 	[multi_line \
 	     "$valnum_re = 0xdeadbeef" \
 	     "$valnum_re = 0xfeedface" \
@@ -212,15 +212,15 @@ proc_with_prefix if_while_breakpoint_command_test {} {
     # This test should alternate between 0xdeadbeef and 0xfeedface two times.
     gdb_test \
 	[multi_line_input \
-	     "while value > 0" \
-	     "  set value -= 1" \
-	     "  if \(value % 2\) == 1" \
-	     "    p/x 0xdeadbeef" \
-	     "  else" \
-	     "    p/x 0xfeedface" \
-	     "  end" \
-	     "end" \
-	     "end"] \
+	     {while value > 0} \
+	     {  set value -= 1} \
+	     {  if (value % 2) == 1} \
+	     {    p/x 0xdeadbeef} \
+	     {  else} \
+	     {    p/x 0xfeedface} \
+	     {  end} \
+	     {end} \
+	     {end}] \
 	"" \
 	"commands part 2"
     gdb_test \
@@ -277,9 +277,9 @@ proc_with_prefix breakpoint_command_test {} {
     gdb_test "break factorial" "Breakpoint.*at.*"
     gdb_test \
 	[multi_line_input \
-	     "commands" \
-	     "  printf \"Now the value is %d\\n\", value" \
-	     "end"] \
+	     {commands} \
+	     {  printf "Now the value is %d\n", value} \
+	     {end}] \
 	"End with.*" \
 	"commands"
     gdb_test "continue" \
@@ -303,15 +303,15 @@ proc_with_prefix user_defined_command_test {} {
     # This test should alternate between 0xdeadbeef and 0xfeedface two times.
     gdb_test \
 	[multi_line_input \
-	     "while \$arg0 > 0" \
-	     "  set \$arg0 -= 1" \
-	     "  if \(\$arg0 % 2\) == 1" \
-	     "    p/x 0xdeadbeef" \
-	     "  else" \
-	     "    p/x 0xfeedface" \
-	     "  end" \
-	     "end" \
-	     "end"] \
+	     {while $arg0 > 0} \
+	     {  set $arg0 -= 1} \
+	     {  if ($arg0 % 2) == 1} \
+	     {    p/x 0xdeadbeef} \
+	     {  else} \
+	     {    p/x 0xfeedface} \
+	     {  end} \
+	     {end} \
+	     {end}] \
 	"" \
 	"enter commands"
 
@@ -319,7 +319,7 @@ proc_with_prefix user_defined_command_test {} {
     set valnum_re "\\\$$decimal"
 
     gdb_test \
-	"mycommand \$foo" \
+	{mycommand $foo} \
 	[multi_line \
 	     "$valnum_re = 0xdeadbeef" \
 	     "$valnum_re = 0xfeedface" \
@@ -438,11 +438,11 @@ proc_with_prefix test_command_prompt_position {} {
     # All this test should do is print 0xdeadbeef once.
     gdb_test \
 	[multi_line_input \
-	     "if value == 1" \
-	     "  p/x 0xfeedface" \
-	     "else" \
-	     "  p/x 0xdeadbeef" \
-	     "end"] \
+	     {if value == 1} \
+	     {  p/x 0xfeedface} \
+	     {else} \
+	     {  p/x 0xdeadbeef} \
+	     {end}] \
 	"$valnum_re = 0xdeadbeef" \
 	"if test"
 
-- 
2.5.5


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/5] Further cleanup/modernization of gdb.base/commands.exp
  2016-11-09 16:37             ` Pedro Alves
@ 2016-11-09 16:41               ` Pedro Alves
  0 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2016-11-09 16:41 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On 11/09/2016 04:37 PM, Pedro Alves wrote:
> On 11/09/2016 04:22 PM, Pedro Alves wrote:
>> On 11/09/2016 04:09 PM, Simon Marchi wrote:
> 
>>> That's not bad.  I was going to suggest using {} instead of "" to get
>>> rid of most backslashes (untested):
>>>
>>> -"\\\$\[0-9\]* = 0xdeadbeef"
>>> +{\$[0-9]* = 0xdeadbeef}
>>
>> I strikes me that I can use {}when building the input strings.  I was
>> already doing it in the new tests added by the following patches, but
>> not in this one, for some reason.  Let me give that a try.
> 
> Like this.  Done throughout for consistency.
> 
> Must stop polishing this.  :-)
> 


> @@ -69,10 +69,10 @@ proc_with_prefix gdbvar_simple_while_test {} {
>      # This test should print 0xfeedface five times.
>      gdb_test \
>  	[multi_line_input \
> -	     "while \$foo > 0" \
> -	     "  p/x 0xfeedface" \
> -	     "  set \$foo -= 1" \
> -	     "end"] \
> +	     {while $foo > 0} \
> +	     {  p/x 0xfeedface} \
> +	     {set $foo -= 1} \
> +	     {end}] \

Fixed this indentation, and squashed it in.  Nth time's the charm.

From c29aa63b7aeb31229aeff5468fc202f3699eb6ac Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 9 Nov 2016 16:38:51 +0000
Subject: [PATCH] Further cleanup/modernization of gdb.base/commands.exp

 - Use multi_line for matching multi-line GDB output.

 - Add a multi_line_input variant of multi_line to build GDB input and
   use it throughout.

   (The two changes above make the tests much more readable, IMO.)

 - Add a new valnum_re global to get rid of the multiple "\\\$\[0-9\]*".

 - Remove gdb_stop_suppressing_tests uses.

 - tighten a few regexps.

 - Replace send_gdb/gdb_expect with gdb_test_multiple and simplify,
   making pass/fail messages the same.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.base/commands.exp (runto_or_return): New
	procedure.
	(gdbvar_simple_if_test, gdbvar_simple_while_test)
	(gdbvar_complex_if_while_test, progvar_simple_if_test)
	(progvar_simple_while_test, progvar_complex_if_while_test)
	(if_while_breakpoint_command_test)
	(infrun_breakpoint_command_test, breakpoint_command_test)
	(user_defined_command_test, watchpoint_command_test)
	(test_command_prompt_position, redefine_hook_test)
	(stray_arg0_test, error_clears_commands_left, redefine_hook_test)
	(redefine_backtrace_test): Use runto_or_return, $valnum_re,
	multi_line_input and multi_line.  Remove gdb_expect and
	gdb_stop_suppressing_tests uses.
	* lib/gdb.exp (valnum_re): New global.
	(multi_line_input): New procedure.
---
 gdb/testsuite/gdb.base/commands.exp | 365 ++++++++++++++++++++++++------------
 gdb/testsuite/lib/gdb.exp           |  13 ++
 2 files changed, 257 insertions(+), 121 deletions(-)

diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index fe2c23a..ee7bd38 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -23,91 +23,183 @@ if { [prepare_for_testing commands.exp commands run.c {debug additional_flags=-D
     return -1
 }
 
+# Run to FUNCTION.  If that fails, issue a FAIL and make the caller
+# return.
+
+proc runto_or_return {function} {
+    if { ![runto factorial] } {
+	fail "cannot run to $function"
+	return -code return
+    }
+}
+
 proc_with_prefix gdbvar_simple_if_test {} {
     global gdb_prompt
+    global valnum_re
 
     gdb_test_no_output "set \$foo = 0" "set foo"
     # All this test should do is print 0xdeadbeef once.
-    gdb_test "if \$foo == 1\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
-	    "\\\$\[0-9\]* = 0xdeadbeef" "#1"
+    gdb_test \
+	[multi_line_input \
+	     {if $foo == 1} \
+	     {  p/x 0xfeedface} \
+	     {else} \
+	     {  p/x 0xdeadbeef} \
+	     {end}] \
+	"$valnum_re = 0xdeadbeef" \
+	"#1"
+
     # All this test should do is print 0xfeedface once.
-    gdb_test "if \$foo == 0\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
-	    "\\\$\[0-9\]* = 0xfeedface" "#2"
+    gdb_test \
+	[multi_line_input \
+	     {if $foo == 0} \
+	     {  p/x 0xfeedface} \
+	     {else} \
+	     {  p/x 0xdeadbeef} \
+	     {end}] \
+	"$valnum_re = 0xfeedface" \
+	"#2"
 }
 
 proc_with_prefix gdbvar_simple_while_test {} {
     global gdb_prompt
+    global valnum_re
 
     gdb_test_no_output "set \$foo = 5" "set foo"
     # This test should print 0xfeedface five times.
-    gdb_test "while \$foo > 0\np/x 0xfeedface\nset \$foo -= 1\nend" \
-	    "\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "#1"
+    gdb_test \
+	[multi_line_input \
+	     {while $foo > 0} \
+	     {  p/x 0xfeedface} \
+	     {  set $foo -= 1} \
+	     {end}] \
+	[multi_line \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface"] \
+	"#1"
 }
 
 proc_with_prefix gdbvar_complex_if_while_test {} {
     global gdb_prompt
+    global valnum_re
 
-    gdb_test_no_output "set \$foo = 4" \
-	"set foo"
+    gdb_test_no_output "set \$foo = 4" "set foo"
     # This test should alternate between 0xdeadbeef and 0xfeedface two times.
-    gdb_test "while \$foo > 0\nset \$foo -= 1\nif \(\$foo % 2\) == 1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend" \
-	    "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "#1"
+    gdb_test \
+	[multi_line_input \
+	     {while $foo > 0} \
+	     {  set $foo -= 1} \
+	     {  if ($foo % 2) == 1} \
+	     {    p/x 0xdeadbeef} \
+	     {  else} \
+	     {    p/x 0xfeedface} \
+	     {  end} \
+	     {end}] \
+	[multi_line \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface"] \
+	"#1"
 }
 
 proc_with_prefix progvar_simple_if_test {} {
     global gdb_prompt
+    global valnum_re
+
+    runto_or_return factorial
 
-    if { ![runto factorial] } then { gdb_suppress_tests; }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=5" ".*" "set value to 5"
+    gdb_test "p value=5" " = 5" "set value to 5"
     # All this test should do is print 0xdeadbeef once.
-    gdb_test "if value == 1\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
-	    "\\\$\[0-9\]* = 0xdeadbeef" \
-	    "#1"
+    gdb_test \
+	[multi_line_input \
+	     {if value == 1} \
+	     {  p/x 0xfeedface} \
+	     {else} \
+	     {  p/x 0xdeadbeef} \
+	     {end}] \
+	"$valnum_re = 0xdeadbeef" \
+	"#1"
+
     # All this test should do is print 0xfeedface once.
-    gdb_test "if value == 5\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
-	    "\\\$\[0-9\]* = 0xfeedface" \
-	    "#2"
-    gdb_stop_suppressing_tests
+    gdb_test \
+	[multi_line_input \
+	     {if value == 5} \
+	     {  p/x 0xfeedface} \
+	     {else} \
+	     {  p/x 0xdeadbeef} \
+	     {end}] \
+	"$valnum_re = 0xfeedface" \
+	"#2"
 }
 
 proc_with_prefix progvar_simple_while_test {} {
     global gdb_prompt
+    global valnum_re
+
+    runto_or_return factorial
 
-    if { ![runto factorial] } then { gdb_suppress_tests }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=5" ".*" "set value to 5"
+    gdb_test "p value=5" " = 5" "set value to 5"
     # This test should print 0xfeedface five times.
-    gdb_test "while value > 0\np/x 0xfeedface\nset value -= 1\nend" \
-	    "\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "#1"
-    gdb_stop_suppressing_tests
+    gdb_test \
+	[multi_line_input \
+	     {while value > 0} \
+	     {  p/x 0xfeedface} \
+	     {  set value -= 1} \
+	     {end}] \
+	[multi_line \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xfeedface"] \
+	"#1"
 }
 
 proc_with_prefix progvar_complex_if_while_test {} {
     global gdb_prompt
+    global valnum_re
+
+    runto_or_return factorial
 
-    if { ![runto factorial] } then { gdb_suppress_tests }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=4" ".*" "set value to 4"
-    # This test should alternate between 0xdeadbeef and 0xfeedface two times.
-    gdb_test "while value > 0\nset value -= 1\nif \(value % 2\) == 1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend" \
-	    "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "#1"
-    gdb_stop_suppressing_tests
+    gdb_test "p value=4" " = 4" "set value to 4"
+    # This test should alternate between 0xdeadbeef and 0xfeedface two
+    # times.
+    gdb_test \
+	[multi_line_input \
+	     {while value > 0} \
+	     {  set value -= 1} \
+	     {  if (value % 2) == 1} \
+	     {    p/x 0xdeadbeef} \
+	     {  else} \
+	     {    p/x 0xfeedface} \
+	     {  end} \
+	     {end}] \
+	[multi_line \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface"] \
+	"#1"
 }
 
 proc_with_prefix if_while_breakpoint_command_test {} {
+    global valnum_re
+
+    runto_or_return factorial
 
-    if { ![runto factorial] } then { gdb_suppress_tests }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=5" ".*" "set value to 5"
+    gdb_test "p value=5" " = 5" "set value to 5"
     delete_breakpoints
     gdb_test "break factorial" "Breakpoint.*at.*" "break factorial"
 
@@ -118,14 +210,28 @@ proc_with_prefix if_while_breakpoint_command_test {} {
     }
 
     # This test should alternate between 0xdeadbeef and 0xfeedface two times.
-    gdb_test "while value > 0\nset value -= 1\nif \(value % 2\) == 1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend\nend" \
-	    "" \
-	    "commands part 2"
-    gdb_test "continue" \
-	    "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "#1"
+    gdb_test \
+	[multi_line_input \
+	     {while value > 0} \
+	     {  set value -= 1} \
+	     {  if (value % 2) == 1} \
+	     {    p/x 0xdeadbeef} \
+	     {  else} \
+	     {    p/x 0xfeedface} \
+	     {  end} \
+	     {end} \
+	     {end}] \
+	"" \
+	"commands part 2"
+    gdb_test \
+	"continue" \
+	[multi_line \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface"] \
+	"#1"
     gdb_test "info break" "while.*set.*if.*p/x.*else.*p/x.*end.*"
-    gdb_stop_suppressing_tests
 }
 
 # Test that we can run the inferior from breakpoint commands.
@@ -135,11 +241,11 @@ proc_with_prefix if_while_breakpoint_command_test {} {
 # subsection "Breakpoint command lists".
 
 proc_with_prefix infrun_breakpoint_command_test {} {
+    runto_or_return factorial
 
-    if { ![runto factorial] } then { gdb_suppress_tests }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=6" ".*" "set value to 6"
+    gdb_test "p value=6" " = 6" "set value to 6"
     delete_breakpoints
     gdb_test "break factorial if value == 5" "Breakpoint.*at.*"
 
@@ -159,29 +265,32 @@ proc_with_prefix infrun_breakpoint_command_test {} {
 
     gdb_test "continue" \
 	"Continuing.*.*.*Breakpoint \[0-9\]*, factorial \\(value=5\\).*at.*\[0-9\]*\[      \]*if \\(value > 1\\) \{.*\[0-9\]*\[      \]*value \\*= factorial \\(value - 1\\);.*"
-
-    gdb_stop_suppressing_tests
 }
 
 proc_with_prefix breakpoint_command_test {} {
+    runto_or_return factorial
 
-    if { ![runto factorial] } then { gdb_suppress_tests; }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
-    gdb_test "p value=6" ".*" "set value to 6"
+    gdb_test "p value=6" " = 6" "set value to 6"
     delete_breakpoints
     gdb_test "break factorial" "Breakpoint.*at.*"
-    gdb_test "commands\nprintf \"Now the value is %d\\n\", value\nend" \
-	"End with.*" "commands"
+    gdb_test \
+	[multi_line_input \
+	     {commands} \
+	     {  printf "Now the value is %d\n", value} \
+	     {end}] \
+	"End with.*" \
+	"commands"
     gdb_test "continue" \
 	    "Breakpoint \[0-9\]*, factorial.*Now the value is 5"
     gdb_test "print value" " = 5"
-    gdb_stop_suppressing_tests
 }
 
 # Test a simple user defined command (with arguments)
 proc_with_prefix user_defined_command_test {} {
     global gdb_prompt
+    global valnum_re
 
     gdb_test_no_output "set \$foo = 4" "set foo"
 
@@ -192,13 +301,31 @@ proc_with_prefix user_defined_command_test {} {
     }
 
     # This test should alternate between 0xdeadbeef and 0xfeedface two times.
-    gdb_test "while \$arg0 > 0\nset \$arg0 -= 1\nif \(\$arg0 % 2\) == 1\np/x 0xdeadbeef\nelse\np/x 0xfeedface\nend\nend\nend" \
-	    "" \
-	    "enter commands"
-
-    gdb_test "mycommand \$foo" \
-	    "\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface\[^\n\]*\n\\\$\[0-9\]* = 0xdeadbeef\[^\n\]*\n\\\$\[0-9\]* = 0xfeedface" \
-	    "execute user-defined command"
+    gdb_test \
+	[multi_line_input \
+	     {while $arg0 > 0} \
+	     {  set $arg0 -= 1} \
+	     {  if ($arg0 % 2) == 1} \
+	     {    p/x 0xdeadbeef} \
+	     {  else} \
+	     {    p/x 0xfeedface} \
+	     {  end} \
+	     {end} \
+	     {end}] \
+	"" \
+	"enter commands"
+
+    global decimal
+    set valnum_re "\\\$$decimal"
+
+    gdb_test \
+	{mycommand $foo} \
+	[multi_line \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface" \
+	     "$valnum_re = 0xdeadbeef" \
+	     "$valnum_re = 0xfeedface"] \
+	"execute user-defined command"
    gdb_test "show user mycommand" \
 	"  while \\\$arg0.*set.*    if \\\(\\\$arg0.*p/x.*    else\[^\n\].*p/x.*    end\[^\n\].*  end\[^\n\].*" \
 	   "display user command"
@@ -233,7 +360,8 @@ proc_with_prefix watchpoint_command_test {} {
 	gdb_test_no_output "set can-use-hw-watchpoints 0" ""
     }
 
-    if { ![runto factorial] } then { return }
+    runto_or_return factorial
+
     delete_breakpoints
 
     # Verify that we can create a watchpoint, and give it a commands
@@ -298,60 +426,44 @@ proc_with_prefix watchpoint_command_test {} {
 
 proc_with_prefix test_command_prompt_position {} {
     global gdb_prompt
+    global valnum_re
+
+    runto_or_return factorial
 
-    if { ![runto factorial] } then { gdb_suppress_tests; }
     # Don't depend upon argument passing, since most simulators don't
     # currently support it.  Bash value variable to be what we want.
     delete_breakpoints
     gdb_test "break factorial" "Breakpoint.*at.*"
     gdb_test "p value=5" ".*" "set value to 5"
     # All this test should do is print 0xdeadbeef once.
-    gdb_test "if value == 1\np/x 0xfeedface\nelse\np/x 0xdeadbeef\nend" \
-	    "\\\$\[0-9\]* = 0xdeadbeef" \
-	    "if test"
-    
+    gdb_test \
+	[multi_line_input \
+	     {if value == 1} \
+	     {  p/x 0xfeedface} \
+	     {else} \
+	     {  p/x 0xdeadbeef} \
+	     {end}] \
+	"$valnum_re = 0xdeadbeef" \
+	"if test"
+
     # Now let's test for the correct position of the '>' in gdb's
     # prompt for commands.  It should be at the beginning of the line,
     # and not after one space.
 
-    send_gdb "commands\n"
-    gdb_expect {
-	-re "Type commands.*End with.*\[\r\n\]>$" { 
-	    send_gdb "printf \"Now the value is %d\\n\", value\n"
-	    gdb_expect {
+    set test "> OK"
+    gdb_test_multiple "commands" $test {
+	-re "Type commands.*End with.*\[\r\n\]>$" {
+	    gdb_test_multiple "printf \"Now the value is %d\\n\", value" $test {
 		-re "^printf.*value\r\n>$" {
-		    send_gdb "end\n"
-		    gdb_expect {
+		    gdb_test_multiple "end" $test {
 			-re "^end\r\n$gdb_prompt $" { 
-			    pass "> OK"
-			}
-			-re ".*$gdb_prompt $" { 
-			    fail "some other message"
-			}
-			timeout  { 
-			    fail "(timeout) 1"
+			    pass $test
 			}
 		    }
 		}
-		-re "^ >$" { fail "> not OK" }
-		-re ".*$gdb_prompt $"   { 
-		    fail "wrong message"
-		}
-		timeout    { 
-		    fail "(timeout) 2"
-		}
 	    }
 	}
-	-re "Type commands.*End with.*\[\r\n\] >$" { 
-	    fail "prompt not OK"
-	}
-	-re ".*$gdb_prompt $" { 
-	    fail "commands"
-	}
-	timeout { fail "(timeout) 3" }
     }
-
-    gdb_stop_suppressing_tests
 }
 
 
@@ -480,20 +592,22 @@ proc_with_prefix temporary_breakpoint_commands {} {
 # Test that GDB can handle $arg0 outside of user functions without
 # crashing.
 proc_with_prefix stray_arg0_test { } {
+    global valnum_re
+
     gdb_test "print \$arg0" \
-	"\\\$\[0-9\]* = void" \
+	"$valnum_re = void" \
 	"#1"
 
     gdb_test "if 1 == 1\nprint \$arg0\nend" \
-	"\\\$\[0-9\]* = void" \
+	"$valnum_re = void" \
 	"#2"
 
     gdb_test "print \$arg0 = 1" \
-	"\\\$\[0-9\]* = 1" \
+	"$valnum_re = 1" \
 	"#3"
 
     gdb_test "print \$arg0" \
-	"\\\$\[0-9\]* = 1" \
+	"$valnum_re = 1" \
 	"#4"
 }
 
@@ -685,7 +799,14 @@ proc_with_prefix error_clears_commands_left {} {
     gdb_test_no_output "end" "main commands 2c"
 
     gdb_run_cmd
-    gdb_test "" "hook-stop1\r\n.*\r\ncmd1\r\nUndefined command: \"errorcommandxy\"\\.  Try \"help\"\\." "cmd1 error"
+    gdb_test \
+	"" \
+	[multi_line \
+	     "hook-stop1" \
+	     ".*" \
+	     "cmd1" \
+	     "Undefined command: \"errorcommandxy\"\\.  Try \"help\"\\."] \
+	"cmd1 error"
 
     gdb_test {echo idle\n} "\r\nidle" "no cmd2"
 }
@@ -693,13 +814,20 @@ proc_with_prefix error_clears_commands_left {} {
 proc_with_prefix redefine_hook_test {} {
     global gdb_prompt
 
-    gdb_test "define one\nend" \
-      "" \
-      "define one"
+    gdb_test \
+	[multi_line_input \
+	     "define one"\
+	     "end"] \
+	"" \
+	"define one"
 
-    gdb_test "define hook-one\necho hibob\\n\nend" \
-      "" \
-      "define hook-one"
+    gdb_test \
+	[multi_line_input \
+	     "define hook-one" \
+	     "echo hibob\\n" \
+	     "end"] \
+	"" \
+	"define hook-one"
 
     set test "redefine one"
     gdb_test_multiple "define one" $test {
@@ -713,13 +841,9 @@ proc_with_prefix redefine_hook_test {} {
 	}
     }
 
-    gdb_test "end" \
-	    "" \
-	    "enter commands for one redefinition"
+    gdb_test "end" "" "enter commands for one redefinition"
 
-    gdb_test "one" \
-	    "hibob" \
-	    "execute one command"
+    gdb_test "one" "hibob" "execute one command"
 }
 
 proc_with_prefix redefine_backtrace_test {} {
@@ -737,16 +861,15 @@ proc_with_prefix redefine_backtrace_test {} {
 	}
     }
 
-    gdb_test "echo hibob\\n\nend" \
-	    "" \
-	    "enter commands"
+    gdb_test \
+	[multi_line_input \
+	     "echo hibob\\n" \
+	     "end"] \
+	"" \
+	"enter commands"
 
-    gdb_test "backtrace" \
-	    "hibob" \
-	    "execute backtrace command"
-    gdb_test "bt" \
-	    "hibob" \
-	    "execute bt command"
+    gdb_test "backtrace" "hibob" "execute backtrace command"
+    gdb_test "bt" "hibob" "execute bt command"
 }
 
 gdbvar_simple_if_test
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 735ed11..e1e9880 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -117,6 +117,10 @@ set octal "\[0-7\]+"
 
 set inferior_exited_re "(\\\[Inferior \[0-9\]+ \\(.*\\) exited)"
 
+# A regular expression that matches a value history number.
+# E.g., $1, $2, etc.
+set valnum_re "\\\$$decimal"
+
 ### Only procedures should come after this point.
 
 #
@@ -6005,5 +6009,14 @@ proc multi_line { args } {
     return [join $args "\r\n"]
 }
 
+# Similar to the above, but while multi_line is meant to be used to
+# match GDB output, this one is meant to be used to build strings to
+# send as GDB input.
+
+proc multi_line_input { args } {
+    return [join $args "\n"]
+}
+
+
 # Always load compatibility stuff.
 load_lib future.exp
-- 
2.5.5


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/5] Further cleanup/modernization of gdb.base/commands.exp
  2016-11-09 16:25             ` Simon Marchi
@ 2016-11-09 18:51               ` Pedro Alves
  0 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2016-11-09 18:51 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On 11/09/2016 04:24 PM, Simon Marchi wrote:
> On 2016-11-09 11:16, Pedro Alves wrote:
>> On 11/09/2016 04:09 PM, Simon Marchi wrote:
>>> On 2016-11-09 10:59, Pedro Alves wrote:
>>>> And I'm wondering whether this below as well would be a good idea,
>>>> or whether it'd obfuscate?  If a good idea, maybe we'd put the new
>>>> variable in gdb.exp.
>>>
>>> ...
>>>
>>>> @@ -45,7 +50,7 @@ proc_with_prefix gdbvar_simple_if_test {} {
>>>>           "else" \
>>>>           "  p/x 0xdeadbeef" \
>>>>           "end"] \
>>>> -    "\\\$\[0-9\]* = 0xdeadbeef" \
>>>> +    "$valnum_re = 0xdeadbeef" \
>>>>      "#1"
>>>
>>>
>>> That's not bad.  I was going to suggest using {} instead of "" to get
>>> rid of most backslashes (untested):
>>>
>>> -"\\\$\[0-9\]* = 0xdeadbeef"
>>> +{\$[0-9]* = 0xdeadbeef}
>>>
>>> but with the variable it looks good as well.
>>>
>>
>> Here's the resulting squashed patch then.  WDYT?
> 
> LGTM.
> 

Thanks for all the review and bouncing off ideas.  I've pushed in
the version using {} throughout.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2016-11-09 18:51 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-09  0:19 [PATCH v2 0/5] Support an "unlimited" number of user-defined arguments Pedro Alves
2016-11-09  0:19 ` [PATCH v2 1/5] gdb/testsuite: Introduce "proc_with_prefix" Pedro Alves
2016-11-09  1:56   ` Simon Marchi
2016-11-09  3:10   ` [PATCH] Make gdb.mi/user-selected-context-sync.exp use proc_with_prefix Simon Marchi
2016-11-09 15:15     ` Pedro Alves
2016-11-09 15:57       ` Simon Marchi
2016-11-09 11:50   ` [PATCH v2 1/5] gdb/testsuite: Introduce "proc_with_prefix" Yao Qi
2016-11-09  0:19 ` [PATCH v2 3/5] Fix PR 20559 - "eval" command and $arg0...$arg9/$argc substitution Pedro Alves
2016-11-09  0:19 ` [PATCH v2 2/5] Further cleanup/modernization of gdb.base/commands.exp Pedro Alves
2016-11-09  3:02   ` Simon Marchi
2016-11-09 15:56     ` Pedro Alves
2016-11-09 15:59       ` Pedro Alves
2016-11-09 16:09         ` Simon Marchi
2016-11-09 16:16           ` Pedro Alves
2016-11-09 16:25             ` Simon Marchi
2016-11-09 18:51               ` Pedro Alves
2016-11-09 16:22           ` Pedro Alves
2016-11-09 16:37             ` Pedro Alves
2016-11-09 16:41               ` Pedro Alves
2016-11-09  0:25 ` [PATCH v2 5/5] Support an "unlimited" number of user-defined arguments Pedro Alves
2016-11-09  0:26 ` [PATCH v2 4/5] Test user-defined gdb commands and arguments stack Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).