This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[patch[ Improve the error messages of tvariable command
- From: Hafiz Abid Qadeer <hafiz_abid at mentor dot com>
- To: <gdb-patches at sourceware dot org>, <palves at redhat dot com>
- Date: Tue, 12 Feb 2013 16:59:32 +0000
- Subject: [patch[ Improve the error messages of tvariable command
Hi All,
This patch improves the error message of tvariable command. The
difference in error messages in various cases is shown below. This
patch was originally written by Pedro. I have updated it to latest code
and regtested. Is it ok?
Regards,
Abid
Before the patch:
(gdb) start
Temporary breakpoint 1 at 0x80483ba: file test.c, line 3.
Starting program: /home/abidh/test
Temporary breakpoint 1, main () at test.c:3
3 int x = 1;
(gdb) list
1 int main()
2 {
3 int x = 1;
4 return 0;
5 }
(gdb) tvar x
Syntax must be $NAME [ = EXPR ]
(gdb) tvar y
No symbol "y" in current context.
(gdb) tvar $
Syntax must be $NAME [ = EXPR ]
(gdb) tvar $1234
Syntax must be $NAME [ = EXPR ]
After the patch:
(gdb) start
Temporary breakpoint 1 at 0x80483ba: file test.c, line 3.
Starting program: /home/abidh/test
Temporary breakpoint 1, main () at test.c:3
3 int x = 1;
(gdb) list
1 int main()
2 {
3 int x = 1;
4 return 0;
5 }
(gdb) tvar x
Name of trace variable should start with '$'
(gdb) tvar y
Name of trace variable should start with '$'
(gdb) tvar $
Must supply a non-empty variable name
(gdb) tvar $1234
$1234 is not a valid trace state variable name
(gdb) tvar $asdf=1=1
Left operand of assignment is not an lvalue.
2013-02-12 Pedro Alves <palves@redhat.com>
Hafiz Abid Qadeer <abidh@codesourcery.com>
gdb/
* tracepoint.h (validate_trace_state_variable_name): Declare.
* tracepoint.c (validate_trace_state_variable_name): New.
(trace_variable_command): Parse the trace state variable's name
without using parse_expression. Do several validations.
* mi/mi-main.c (mi_cmd_trace_define_variable): Don't parse the
trace state variable's name with parse_expression. Validate it.
gdb/testsuite/
* gdb.trace/tsv.exp: Adjust tests, and add a few more.
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 28de126aa..37294e0 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2357,7 +2357,6 @@ void
mi_cmd_trace_define_variable (char *command, char **argv, int argc)
{
struct expression *expr;
- struct cleanup *back_to;
LONGEST initval = 0;
struct trace_state_variable *tsv;
char *name = 0;
@@ -2365,19 +2364,11 @@ mi_cmd_trace_define_variable (char *command, char **argv, int argc)
if (argc != 1 && argc != 2)
error (_("Usage: -trace-define-variable VARIABLE [VALUE]"));
- expr = parse_expression (argv[0]);
- back_to = make_cleanup (xfree, expr);
-
- if (expr->nelts == 3 && expr->elts[0].opcode == OP_INTERNALVAR)
- {
- struct internalvar *intvar = expr->elts[1].internalvar;
-
- if (intvar)
- name = internalvar_name (intvar);
- }
+ name = argv[0];
+ if (*name++ != '$')
+ error (_("Name of trace variable should start with '$'"));
- if (!name || *name == '\0')
- error (_("Invalid name of trace variable"));
+ validate_trace_state_variable_name (name);
tsv = find_trace_state_variable (name);
if (!tsv)
@@ -2387,8 +2378,6 @@ mi_cmd_trace_define_variable (char *command, char **argv, int argc)
initval = value_as_long (parse_and_eval (argv[1]));
tsv->initial_value = initval;
-
- do_cleanups (back_to);
}
void
diff --git a/gdb/testsuite/gdb.trace/tsv.exp b/gdb/testsuite/gdb.trace/tsv.exp
index 4ea930f..47d66ad 100644
--- a/gdb/testsuite/gdb.trace/tsv.exp
+++ b/gdb/testsuite/gdb.trace/tsv.exp
@@ -46,14 +46,30 @@ gdb_test "tvariable \$tvar3 = 1234567000000" \
"Trace state variable \\\$tvar3 now has initial value 1234567000000." \
"Init trace state variable to a 64-bit value"
+gdb_test "tvariable $" \
+ "Must supply a non-empty variable name" \
+ "tvariable syntax error, not empty variable name"
+
gdb_test "tvariable main" \
- "Syntax must be \\\$NAME \\\[ = EXPR \\\]" \
+ "Name of trace variable should start with '\\\$'" \
"tvariable syntax error, bad name"
+gdb_test "tvariable \$\$" \
+ "Syntax must be \\\$NAME \\\[ = EXPR \\\]" \
+ "tvariable syntax error, bad name 2"
+
+gdb_test "tvariable \$123" \
+ "\\\$123 is not a valid trace state variable name" \
+ "tvariable syntax error, bad name 3"
+
gdb_test "tvariable \$tvar1 - 93" \
"Syntax must be \\\$NAME \\\[ = EXPR \\\]" \
"tvariable syntax error, not an assignment"
+gdb_test "tvariable \$tvar0 = 1 = 1" \
+ "Left operand of assignment is not an lvalue\." \
+ "tvariable creation fails with invalid expression"
+
gdb_test "info tvariables" \
"Name\[\t \]+Initial\[\t \]+Current.*
\\\$tvar1\[\t \]+0\[\t \]+<undefined>.*
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index b45863e..540665d 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -362,50 +362,67 @@ delete_trace_state_variable (const char *name)
warning (_("No trace variable named \"$%s\", not deleting"), name);
}
+/* Throws an error if NAME is not valid syntax for a trace state
+ variable's name. */
+
+void
+validate_trace_state_variable_name (const char *name)
+{
+ const char *p;
+
+ if (*name == '\0')
+ error (_("Must supply a non-empty variable name"));
+
+ /* All digits in the name is reserved for value history
+ references. */
+ for (p = name; isdigit (*p); p++)
+ ;
+ if (*p == '\0')
+ error (_("$%s is not a valid trace state variable name"), name);
+
+ for (p = name; isalnum (*p) || *p == '_'; p++)
+ ;
+ if (*p != '\0')
+ error (_("$%s is not a valid trace state variable name"), name);
+}
+
/* The 'tvariable' command collects a name and optional expression to
evaluate into an initial value. */
static void
trace_variable_command (char *args, int from_tty)
{
- struct expression *expr;
struct cleanup *old_chain;
- struct internalvar *intvar = NULL;
LONGEST initval = 0;
struct trace_state_variable *tsv;
+ char *name, *p;
if (!args || !*args)
- error_no_arg (_("trace state variable name"));
+ error_no_arg (_("Syntax is $NAME [ = EXPR ]"));
- /* All the possible valid arguments are expressions. */
- expr = parse_expression (args);
- old_chain = make_cleanup (free_current_contents, &expr);
+ /* Only allow two syntaxes; "$name" and "$name=value". */
+ p = skip_spaces (args);
- if (expr->nelts == 0)
- error (_("No expression?"));
+ if (*p++ != '$')
+ error (_("Name of trace variable should start with '$'"));
- /* Only allow two syntaxes; "$name" and "$name=value". */
- if (expr->elts[0].opcode == OP_INTERNALVAR)
- {
- intvar = expr->elts[1].internalvar;
- }
- else if (expr->elts[0].opcode == BINOP_ASSIGN
- && expr->elts[1].opcode == OP_INTERNALVAR)
- {
- intvar = expr->elts[2].internalvar;
- initval = value_as_long (evaluate_subexpression_type (expr, 4));
- }
- else
+ name = p;
+ while (isalnum (*p) || *p == '_')
+ p++;
+ name = savestring (name, p - name);
+ old_chain = make_cleanup (xfree, name);
+
+ p = skip_spaces (p);
+ if (*p != '=' && *p != '\0')
error (_("Syntax must be $NAME [ = EXPR ]"));
- if (!intvar)
- error (_("No name given"));
+ validate_trace_state_variable_name (name);
- if (strlen (internalvar_name (intvar)) <= 0)
- error (_("Must supply a non-empty variable name"));
+ if (*p == '=')
+ initval = value_as_long (parse_and_eval (++p));
/* If the variable already exists, just change its initial value. */
- tsv = find_trace_state_variable (internalvar_name (intvar));
+ tsv = find_trace_state_variable (name);
if (tsv)
{
if (tsv->initial_value != initval)
@@ -421,7 +438,7 @@ trace_variable_command (char *args, int from_tty)
}
/* Create a new variable. */
- tsv = create_trace_state_variable (internalvar_name (intvar));
+ tsv = create_trace_state_variable (name);
tsv->initial_value = initval;
observer_notify_tsv_created (tsv);
diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
index 2e1d83a..61c7212 100644
--- a/gdb/tracepoint.h
+++ b/gdb/tracepoint.h
@@ -246,6 +246,7 @@ extern void validate_actionline (char **, struct breakpoint *);
extern void end_actions_pseudocommand (char *args, int from_tty);
extern void while_stepping_pseudocommand (char *args, int from_tty);
+extern void validate_trace_state_variable_name (const char *name);
extern struct trace_state_variable *find_trace_state_variable (const char *name);
extern struct trace_state_variable *create_trace_state_variable (const char *name);