Patchwork [5/5] Accept convenience variable in commands -break-passcount and -break-commands

login
register
mail settings
Submitter Yao Qi
Date March 13, 2014, 2:32 a.m.
Message ID <1394677950-4054-6-git-send-email-yao@codesourcery.com>
Download mbox | patch
Permalink /patch/61/
State New
Headers show

Comments

Yao Qi - March 13, 2014, 2:32 a.m.
Hi,
When I modify a MI test case to avoid hard code breakpoint number in
the testcase, commands -break-passcount and -break-commands only
accepts numbers and rejects convenience variables, such as $bpnum, as
invalid input.

This patch teaches these commands to accept convenience variables, and
avoid hard code breakpoint number in test case.

I add a NEWS entry since it is user visible.  However, I didn't update
the manual because manual doesn't mention that commands 'enable' 'disable'
accept convenience variable explicitly, likewise, doesn't mention this
for MI commands.  Some test cases are updated to replace hard code
number with convenience variables.

gdb:

2014-03-13  Yao Qi  <yao@codesourcery.com>

	* cli/cli-utils.c (get_number_quiet): New function.
	* cli/cli-utils.h (get_number_quiet): Declare.
	* mi/mi-cmd-break.c: Include "cli/cli-utils.h".
	(mi_cmd_break_passcount): Use get_number_quiet instead of atoi.
	(mi_cmd_break_commands): Likewise.
	* NEWS: Mention that commands -break-passcount and
	-break-commands accept convenience variable.

gdb/testsuite:

2014-03-13  Yao Qi  <yao@codesourcery.com>

	* gdb.mi/mi-break.exp: Test the error messages to invalid
	input for -break-commands.
	* gdb.mi/mi-breakpoint-changed.exp (test_insert_delete_modify):
	Test command -break-passcount accepts convenience variable.
	* gdb.trace/mi-trace-frame-collected.exp: Test command
	-break-commands accepts convenience variable.
	* gdb.trace/mi-trace-unavailable.exp: Likewise.
---
 gdb/NEWS                                           |    5 ++++
 gdb/cli/cli-utils.c                                |    8 ++++++
 gdb/cli/cli-utils.h                                |    4 +++
 gdb/mi/mi-cmd-break.c                              |   26 +++++++++++++++----
 gdb/testsuite/gdb.mi/mi-break.exp                  |    8 ++++++
 gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp     |    2 +
 .../gdb.trace/mi-trace-frame-collected.exp         |    4 +-
 gdb/testsuite/gdb.trace/mi-trace-unavailable.exp   |    4 +-
 8 files changed, 51 insertions(+), 10 deletions(-)
Eli Zaretskii - March 14, 2014, 8:22 a.m.
> From: Yao Qi <yao@codesourcery.com>
> Date: Thu, 13 Mar 2014 10:32:30 +0800
> 
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -95,6 +95,11 @@ PowerPC64 GNU/Linux little-endian	powerpc64le-*-linux*
>    and "assf"), have been deprecated.  Use the "sharedlibrary" command, or
>    its alias "share", instead.
>  
> +* MI changes
> +
> +  ** The commands -break-passcount and -break-commands accept convenience
> +     variable as breakpoint number.
> +

Please say "variables" and "numbers", in plural.

Otherwise, this part of the patch is OK.

Thanks.

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index 2a384ba..f667538 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -95,6 +95,11 @@  PowerPC64 GNU/Linux little-endian	powerpc64le-*-linux*
   and "assf"), have been deprecated.  Use the "sharedlibrary" command, or
   its alias "share", instead.
 
+* MI changes
+
+  ** The commands -break-passcount and -break-commands accept convenience
+     variable as breakpoint number.
+
 *** Changes in GDB 7.7
 
 * Improved support for process record-replay and reverse debugging on
diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index 53211a4..b906a0f 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -125,6 +125,14 @@  print_get_number_status (enum get_number_status status)
 /* See documentation in cli-utils.h.  */
 
 enum get_number_status
+get_number_quiet (char **pp, int *number)
+{
+  return get_number_trailer (pp, '\0', number);
+}
+
+/* See documentation in cli-utils.h.  */
+
+enum get_number_status
 get_number (char **pp, int *number)
 {
   enum get_number_status status;
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index eee23f4..fc938ec 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -51,6 +51,10 @@  enum get_number_status
 
 extern enum get_number_status get_number (char **pp, int *number);
 
+/* Similar to get_number, but it doesn't print messages.  */
+
+extern enum get_number_status get_number_quiet (char **pp, int *number);
+
 /* An object of this type is passed to get_number_or_range.  It must
    be initialized by calling init_number_or_range.  This type is
    defined here so that it can be stack-allocated, but all members
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 29e3fb3..4ef8eb5 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -32,6 +32,7 @@ 
 #include "mi-cmd-break.h"
 #include "gdb_obstack.h"
 #include <ctype.h>
+#include "cli/cli-utils.h"
 
 enum
   {
@@ -330,12 +331,18 @@  mi_cmd_break_passcount (char *command, char **argv, int argc)
   int n;
   int p;
   struct tracepoint *t;
+  char *s;
 
   if (argc != 2)
     error (_("Usage: tracepoint-number passcount"));
 
-  n = atoi (argv[0]);
-  p = atoi (argv[1]);
+  s = argv[0];
+  if (get_number_quiet (&s, &n) != GET_NUMBER_OK)
+    error (_("Invalid tracepoint number \"%s\"."), argv[0]);
+
+  s = argv[1];
+  if (get_number_quiet (&s, &p) != GET_NUMBER_OK)
+    error (_("Invalid passcount number \"%s\"."), argv[1]);
   t = get_tracepoint (n);
 
   if (t)
@@ -437,18 +444,25 @@  void
 mi_cmd_break_commands (char *command, char **argv, int argc)
 {
   struct command_line *break_command;
-  char *endptr;
+
   int bnum;
   struct breakpoint *b;
+  char *p = argv[0];
+  enum get_number_status status;
 
   if (argc < 1)
     error (_("USAGE: %s <BKPT> [<COMMAND> [<COMMAND>...]]"), command);
 
-  bnum = strtol (argv[0], &endptr, 0);
-  if (endptr == argv[0])
+  status = get_number_quiet (&p, &bnum);
+
+  if (status == GET_NUMBER_E_HISTORY_VALUE_TYPE)
+    error (_("history value must have integer type"));
+  else if (status == GET_NUMBER_E_CONVENIENCE_TYPE)
+    error (_("convenience variable must have integer value"));
+  else if (status == GET_NUMBER_E_NOT_NUMBER)
     error (_("breakpoint number argument \"%s\" is not a number."),
 	   argv[0]);
-  else if (*endptr != '\0')
+  else if (status == GET_NUMBER_E_TRAILING_JUNK)
     error (_("junk at the end of breakpoint number argument \"%s\"."),
 	   argv[0]);
 
diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
index a9064fd..ea8fb37 100644
--- a/gdb/testsuite/gdb.mi/mi-break.exp
+++ b/gdb/testsuite/gdb.mi/mi-break.exp
@@ -232,6 +232,14 @@  proc test_breakpoint_commands {} {
     mi_create_breakpoint "basics.c:callee2" 7 keep callee2 ".*basics.c" $line_callee2_body $hex \
              "breakpoint commands: insert breakpoint at basics.c:callee2"
 
+    mi_gdb_test "-break-commands xyz \"print 10\" \"continue\"" \
+	"\\^error,msg=\"breakpoint number argument \\\\\"xyz\\\\\" is not a number\..*" \
+	"breakpoint commands: not a number"
+
+    mi_gdb_test "-break-commands 7.7 \"print 10\" \"continue\"" \
+	"\\^error,msg=\"junk at the end of breakpoint number argument \\\\\"7\.7\\\\\".*" \
+	"breakpoint commands: junk at the end"
+
     mi_gdb_test "-break-commands 7 \"print 10\" \"continue\"" \
         "\\^done" \
         "breakpoint commands: set commands"
diff --git a/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp b/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp
index aa991cf..6049d2c 100644
--- a/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp
+++ b/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp
@@ -141,6 +141,8 @@  proc test_insert_delete_modify { } {
     # notification.
     mi_gdb_test "-break-passcount 4 1" "\\^done" \
 	"-break-passcount 4 1"
+    mi_gdb_test "-break-passcount \$tpnum 2" "\\^done" \
+	"-break-passcount \$tpnum 2"
 
     # Delete some breakpoints and verify that '=breakpoint-deleted
     # notification is correctly emitted.
diff --git a/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp b/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp
index e0f5f8d..9b1c638 100644
--- a/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp
+++ b/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp
@@ -63,7 +63,7 @@  if [is_amd64_regs_target] {
     return -1
 }
 
-mi_gdb_test "-break-commands 3 \"collect gdb_char_test\" \"collect gdb_union1_test\" \"collect gdb_struct1_test.l\" \"collect gdb_arr_test\[0\]\" \"collect $${pcreg}\" \"teval \$tsv += 1\" \"collect \$tsv\" \"end\" " \
+mi_gdb_test "-break-commands \$bpnum \"collect gdb_char_test\" \"collect gdb_union1_test\" \"collect gdb_struct1_test.l\" \"collect gdb_arr_test\[0\]\" \"collect $${pcreg}\" \"teval \$tsv += 1\" \"collect \$tsv\" \"end\" " \
     {\^done} "set action"
 
 mi_gdb_test "-break-insert -a gdb_c_test" \
@@ -73,7 +73,7 @@  mi_gdb_test "-break-insert -a gdb_c_test" \
 # Define an action.
 # Collect a global variable to be sure no registers are collected
 # except PC.
-mi_gdb_test "-break-commands 4 \"collect gdb_char_test\" \"end\" " \
+mi_gdb_test "-break-commands \$bpnum \"collect gdb_char_test\" \"end\" " \
     {\^done} "set action on tracepoint 4"
 
 mi_gdb_test "-trace-start" {.*\^done} "trace start"
diff --git a/gdb/testsuite/gdb.trace/mi-trace-unavailable.exp b/gdb/testsuite/gdb.trace/mi-trace-unavailable.exp
index 6dd0415..4f5dd22 100644
--- a/gdb/testsuite/gdb.trace/mi-trace-unavailable.exp
+++ b/gdb/testsuite/gdb.trace/mi-trace-unavailable.exp
@@ -49,7 +49,7 @@  mi_gdb_test "-break-insert -a bar" \
     "insert tracepoint on bar"
 
 # Define an action.
-mi_gdb_test "-break-commands 3 \"collect array\" \"collect j\" \"end\" " \
+mi_gdb_test "-break-commands \$bpnum \"collect array\" \"collect j\" \"end\" " \
     {\^done} "set action"
 
 mi_gdb_test "-break-insert -a foo" \
@@ -57,7 +57,7 @@  mi_gdb_test "-break-insert -a foo" \
     "insert tracepoint on foo"
 
 # Collect 'main' to make sure no registers are collected except PC.
-mi_gdb_test "-break-commands 4 \"collect main\" \"end\" " \
+mi_gdb_test "-break-commands \$bpnum \"collect main\" \"end\" " \
     {\^done} "set action on tracepoint 4"
 
 mi_gdb_test "-trace-start" {.*\^done} "trace start"