This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [MI tracepoints 6/9] trace variable commands


On Monday 15 March 2010 21:00:38 Pedro Alves wrote:

> On Sunday 14 March 2010 08:58:36, Vladimir Prus wrote:
> > +  name = argv[0];
> > +  if (name[0] != '$')
> > +    error (_("Name of trace variable should start with '$'"));
> > +  ++name;
> > +
> > +  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);
> > +    }
> > +
> > +  if (!name || *name == '\0')
> > +    error (_("Invalid name of trace variable"));
> 
> Waitaminute.  Is there a merge error here?
> 
> Repeating the snippet:
> 
> > +  name = argv[0];
> > +  if (name[0] != '$')
> > +    error (_("Name of trace variable should start with '$'"));
> > +  ++name;
> > +
> 
> I think this whole bit above shouldn't be here.

Yes, it's a merge error.

> > +  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);  <<<<<<<< (1)
> > +    }
> > +
> > +  if (!name || *name == '\0')
> > +    error (_("Invalid name of trace variable"));
> 
> Otherwise, it looks like you can get here with an invalid name, if
> the expression did parse sucessfully, but (1) wasn't reached at all.

At least after removing the unnecessary bit, if (!) is not executed,
name will be NULL -- which seems right behaviour.


Thanks,

-- 
Vladimir Prus
CodeSourcery
vladimir@codesourcery.com
(650) 331-3385 x722

commit 1b7506f06e31d6ae934fc50e73eb1d8bde3ccd28
Author: vladimir <vladimir@e7755896-6108-0410-9592-8049d3e74e28>
Date:   Thu Aug 6 07:50:47 2009 +0000

    -trace-define-variable and -trace-list-variables.
    
    	* tracepoint.c (create_trace_state_variable): Make
    	private copy of name, as opposed to assuming the
    	pointer lives forever.
    	(tvariables_info_1): New.
    	(tvariables_info): Use the above.
    	* tracepoint.h (create_trace_state_variable, tvariables_info_1):
    	Declare.
    	* mi/mi-cmds.c (mi_cmds): Register -trace-define-variable
    	and -trace-list-variables.
    	* mi/mi-cmds.h (mi_cmd_trace_define_variable)
    	(mi_cmd_trace_list_variables): New.
    	* mi/mi-main.c (mi_cmd_trace_define_variable)
    	(mi_cmd_trace_list_variables): New.

diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 6b260fc..a07ee3b 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -106,6 +106,8 @@ struct mi_cmd mi_cmds[] =
   { "thread-info", { NULL, 0 }, mi_cmd_thread_info },
   { "thread-list-ids", { NULL, 0 }, mi_cmd_thread_list_ids},
   { "thread-select", { NULL, 0 }, mi_cmd_thread_select},
+  { "trace-define-variable", { NULL, 0 }, mi_cmd_trace_define_variable },
+  { "trace-list-variables", { NULL, 0 }, mi_cmd_trace_list_variables },
   { "trace-start", { NULL, 0 }, mi_cmd_trace_start },
   { "trace-status", { NULL, 0 }, mi_cmd_trace_status },
   { "trace-stop", { NULL, 0 }, mi_cmd_trace_stop },
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index b5ff61f..dc2b2c6 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -89,6 +89,8 @@ extern mi_cmd_argv_ftype mi_cmd_target_file_delete;
 extern mi_cmd_argv_ftype mi_cmd_thread_info;
 extern mi_cmd_argv_ftype mi_cmd_thread_list_ids;
 extern mi_cmd_argv_ftype mi_cmd_thread_select;
+extern mi_cmd_argv_ftype mi_cmd_trace_define_variable;
+extern mi_cmd_argv_ftype mi_cmd_trace_list_variables;
 extern mi_cmd_argv_ftype mi_cmd_trace_start;
 extern mi_cmd_argv_ftype mi_cmd_trace_status;
 extern mi_cmd_argv_ftype mi_cmd_trace_stop;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 5aebc50..1f1c014 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2082,6 +2082,52 @@ print_diff (struct mi_timestamp *start, struct mi_timestamp *end)
   }
 
 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;
+
+  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);
+    }
+
+  if (!name || *name == '\0')
+    error (_("Invalid name of trace variable"));
+
+  tsv = find_trace_state_variable (name);
+  if (!tsv)
+    tsv = create_trace_state_variable (name);
+
+  if (argc == 2)
+    initval = value_as_long (parse_and_eval (argv[1]));
+
+  tsv->initial_value = initval;
+
+  do_cleanups (back_to);
+}
+
+void
+mi_cmd_trace_list_variables (char *command, char **argv, int argc)
+{
+  if (argc != 0)
+    error (_("-trace-list-variables: no arguments are allowed"));
+
+  tvariables_info_1 ();
+}
+
+void
 mi_cmd_trace_start (char *command, char **argv, int argc)
 {
   start_tracing ();
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index b7fae78..8e55d82 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -271,7 +271,7 @@ create_trace_state_variable (const char *name)
   struct trace_state_variable tsv;
 
   memset (&tsv, 0, sizeof (tsv));
-  tsv.name = name;
+  tsv.name = xstrdup (name);
   tsv.number = next_tsv_number++;
   return VEC_safe_push (tsv_s, tvariables, &tsv);
 }
@@ -300,6 +300,7 @@ delete_trace_state_variable (const char *name)
   for (ix = 0; VEC_iterate (tsv_s, tvariables, ix, tsv); ++ix)
     if (strcmp (name, tsv->name) == 0)
       {
+	xfree ((void *)tsv->name);
 	VEC_unordered_remove (tsv_s, tvariables, ix);
 	return;
       }
@@ -401,17 +402,15 @@ delete_trace_variable_command (char *args, int from_tty)
   dont_repeat ();
 }
 
-/* List all the trace state variables.  */
-
-static void
-tvariables_info (char *args, int from_tty)
+void
+tvariables_info_1 (void)
 {
   struct trace_state_variable *tsv;
   int ix;
-  char *reply;
-  ULONGEST tval;
+  int count = 0;
+  struct cleanup *back_to;
 
-  if (VEC_length (tsv_s, tvariables) == 0)
+  if (VEC_length (tsv_s, tvariables) == 0 && !ui_out_is_mi_like_p (uiout))
     {
       printf_filtered (_("No trace state variables.\n"));
       return;
@@ -422,24 +421,56 @@ tvariables_info (char *args, int from_tty)
     tsv->value_known = target_get_trace_state_variable_value (tsv->number,
 							      &(tsv->value));
 
-  printf_filtered (_("Name\t\t  Initial\tCurrent\n"));
+  back_to = make_cleanup_ui_out_table_begin_end (uiout, 3,
+                                                 count, "trace-variables");
+  ui_out_table_header (uiout, 15, ui_left, "name", "Name");
+  ui_out_table_header (uiout, 11, ui_left, "initial", "Initial");
+  ui_out_table_header (uiout, 11, ui_left, "current", "Current");
+
+  ui_out_table_body (uiout);
 
   for (ix = 0; VEC_iterate (tsv_s, tvariables, ix, tsv); ++ix)
     {
-      printf_filtered ("$%s", tsv->name);
-      print_spaces_filtered (17 - strlen (tsv->name), gdb_stdout);
-      printf_filtered ("%s ", plongest (tsv->initial_value));
-      print_spaces_filtered (11 - strlen (plongest (tsv->initial_value)), gdb_stdout);
+      struct cleanup *back_to2;
+      char *c;
+      char *name;
+
+      back_to2 = make_cleanup_ui_out_tuple_begin_end (uiout, "variable");
+
+      name = concat ("$", tsv->name, NULL);
+      make_cleanup (xfree, name);
+      ui_out_field_string (uiout, "name", name);
+      ui_out_field_string (uiout, "initial", plongest (tsv->initial_value));
+
       if (tsv->value_known)
-	printf_filtered ("  %s", plongest (tsv->value));
+        c = plongest (tsv->value);
+      else if (ui_out_is_mi_like_p (uiout))
+        /* For MI, we prefer not to use magic string constants, but rather
+           omit the field completely.  The difference between unknown and
+           undefined does not seem important enough to represent.  */
+        c = NULL;
       else if (current_trace_status ()->running || traceframe_number >= 0)
 	/* The value is/was defined, but we don't have it.  */
-	printf_filtered (_("  <unknown>"));
+        c = "<unknown>";
       else
 	/* It is not meaningful to ask about the value.  */
-	printf_filtered (_("  <undefined>"));
-      printf_filtered ("\n");
+        c = "<undefined>";
+      if (c)
+        ui_out_field_string (uiout, "current", c);
+      ui_out_text (uiout, "\n");
+
+      do_cleanups (back_to2);
     }
+
+  do_cleanups (back_to);
+}
+
+/* List all the trace state variables.  */
+
+static void
+tvariables_info (char *args, int from_tty)
+{
+  tvariables_info_1 ();
 }
 
 /* ACTIONS functions: */
diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
index 2e9193c..d03f09a 100644
--- a/gdb/tracepoint.h
+++ b/gdb/tracepoint.h
@@ -145,6 +145,7 @@ extern void end_actions_pseudocommand (char *args, int from_tty);
 extern void while_stepping_pseudocommand (char *args, int from_tty);
 
 extern struct trace_state_variable *find_trace_state_variable (const char *name);
+extern struct trace_state_variable *create_trace_state_variable (const char *name);
 
 extern void parse_trace_status (char *line, struct trace_status *ts);
 
@@ -162,4 +163,6 @@ extern void stop_tracing (void);
 
 extern void trace_status_mi (int on_stop);
 
+extern void tvariables_info_1 (void);
+
 #endif	/* TRACEPOINT_H */

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]