Remove VEC(tsv_s), use std::vector instead

This patch removes VEC(tsv_s), using an std::vector instead.  I C++ified
trace_state_variable a bit in the process, using std::string for the
name.  I also thought it would be nicer to pass a const reference to
target_download_trace_state_variable, since we know it will never be
NULL.  This highlighted that the make-target-delegates script didn't
handle references well, so I adjusted this as well.  It will surely be
useful in the future.

gdb/ChangeLog:

	* tracepoint.h (struct trace_state_variable): Add constructor.
	<name>: Change type to std::string.
	* tracepoint.c (tsv_s): Remove.
	(DEF_VEC_O(tsv_s)): Remove.
	(tvariables): Change to std::vector.
	(create_trace_state_variable): Adjust to std::vector.
	(find_trace_state_variable): Likewise.
	(find_trace_state_variable_by_number): Likewise.
	(delete_trace_state_variable): Likewise.
	(trace_variable_command): Adjust to std::string.
	(delete_trace_variable_command): Likewise.
	(tvariables_info_1): Adjust to std::vector.
	(save_trace_state_variables): Likewise.
	(start_tracing): Likewise.
	(merge_uploaded_trace_state_variables): Adjust to std::vector
	and std::string.
	* target.h (struct target_ops)
	<to_download_trace_state_variable>: Pass reference to
	trace_state_variable.
	* target-debug.h (target_debug_print_const_trace_state_variable_r): New.
	* target-delegates.c: Re-generate.
	* mi/mi-interp.c (mi_tsv_created): Adjust to std::string.
	(mi_tsv_deleted): Likewise.
	* mi/mi-main.c (mi_cmd_trace_frame_collected): Likewise.
	* remote.c (remote_download_trace_state_variable): Change
	pointer to reference and adjust.
	* make-target-delegates (parse_argtypes): Handle references.
	(write_function_header): Likewise.
	(munge_type): Likewise.

gdb/ChangeLog

@@ -1,3 +1,35 @@
+2018-04-09  Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* tracepoint.h (struct trace_state_variable): Add constructor.
+	<name>: Change type to std::string.
+	* tracepoint.c (tsv_s): Remove.
+	(DEF_VEC_O(tsv_s)): Remove.
+	(tvariables): Change to std::vector.
+	(create_trace_state_variable): Adjust to std::vector.
+	(find_trace_state_variable): Likewise.
+	(find_trace_state_variable_by_number): Likewise.
+	(delete_trace_state_variable): Likewise.
+	(trace_variable_command): Adjust to std::string.
+	(delete_trace_variable_command): Likewise.
+	(tvariables_info_1): Adjust to std::vector.
+	(save_trace_state_variables): Likewise.
+	(start_tracing): Likewise.
+	(merge_uploaded_trace_state_variables): Adjust to std::vector
+	and std::string.
+	* target.h (struct target_ops)
+	<to_download_trace_state_variable>: Pass reference to
+	trace_state_variable.
+	* target-debug.h (target_debug_print_const_trace_state_variable_r): New.
+	* target-delegates.c: Re-generate.
+	* mi/mi-interp.c (mi_tsv_created): Adjust to std::string.
+	(mi_tsv_deleted): Likewise.
+	* mi/mi-main.c (mi_cmd_trace_frame_collected): Likewise.
+	* remote.c (remote_download_trace_state_variable): Change
+	pointer to reference and adjust.
+	* make-target-delegates (parse_argtypes): Handle references.
+	(write_function_header): Likewise.
+	(munge_type): Likewise.
+
 2018-04-09  Simon Marchi  <simon.marchi@polymtl.ca>
 
 	* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add

gdb/make-target-delegates

@@ -115,7 +115,7 @@ sub parse_argtypes($) {
     foreach $iter (@typelist) {
 	if ($iter =~ m/^(enum\s+${SYMBOL}\s*)(${SYMBOL})?$/) {
 	    $onetype = $1;
-	} elsif ($iter =~ m/^(.*(enum\s+)?${SYMBOL}.*(\s|\*))${SYMBOL}+$/) {
+	} elsif ($iter =~ m/^(.*(enum\s+)?${SYMBOL}.*(\s|\*|&))${SYMBOL}+$/) {
 	    $onetype = $1;
 	} elsif ($iter eq 'void') {
 	    next;
@@ -151,7 +151,7 @@ sub write_function_header($$@) {
 
 	$val =~ s/$TARGET_DEBUG_PRINTER//;
 
-	if ($iter !~ m,\*$,) {
+	if ($iter !~ m,(\*|&)$,) {
 	    $val .= ' ';
 	}
 
@@ -234,6 +234,7 @@ sub munge_type($) {
 	($result = $typename) =~ s/\s+$//;
 	$result =~ s/[ ()<>:]/_/g;
 	$result =~ s/[*]/p/g;
+	$result =~ s/&/r/g;
 
 	# Identifers with double underscores are reserved to the C++
 	# implementation.

gdb/mi/mi-interp.c

@@ -760,7 +760,7 @@ mi_tsv_created (const struct trace_state_variable *tsv)
 
       fprintf_unfiltered (mi->event_channel, "tsv-created,"
 			  "name=\"%s\",initial=\"%s\"\n",
-			  tsv->name, plongest (tsv->initial_value));
+			  tsv->name.c_str (), plongest (tsv->initial_value));
 
       gdb_flush (mi->event_channel);
     }
@@ -783,7 +783,7 @@ mi_tsv_deleted (const struct trace_state_variable *tsv)
 
       if (tsv != NULL)
 	fprintf_unfiltered (mi->event_channel, "tsv-deleted,"
-			    "name=\"%s\"\n", tsv->name);
+			    "name=\"%s\"\n", tsv->name.c_str ());
       else
 	fprintf_unfiltered (mi->event_channel, "tsv-deleted\n");
 

gdb/mi/mi-main.c

@@ -2665,7 +2665,7 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
 
 	if (tsv != NULL)
 	  {
-	    uiout->field_fmt ("name", "$%s", tsv->name);
+	    uiout->field_fmt ("name", "$%s", tsv->name.c_str ());
 
 	    tsv->value_known = target_get_trace_state_variable_value (tsv->number,
 								      &tsv->value);

gdb/remote.c

@@ -12448,18 +12448,18 @@ remote_can_download_tracepoint (struct target_ops *self)
 
 static void
 remote_download_trace_state_variable (struct target_ops *self,
-				      struct trace_state_variable *tsv)
+				      const trace_state_variable &tsv)
 {
   struct remote_state *rs = get_remote_state ();
   char *p;
 
   xsnprintf (rs->buf, get_remote_packet_size (), "QTDV:%x:%s:%x:",
-	     tsv->number, phex ((ULONGEST) tsv->initial_value, 8),
+	     tsv.number, phex ((ULONGEST) tsv.initial_value, 8),
-	     tsv->builtin);
+	     tsv.builtin);
   p = rs->buf + strlen (rs->buf);
-  if ((p - rs->buf) + strlen (tsv->name) * 2 >= get_remote_packet_size ())
+  if ((p - rs->buf) + tsv.name.length () * 2 >= get_remote_packet_size ())
     error (_("Trace state variable name too long for tsv definition packet"));
-  p += 2 * bin2hex ((gdb_byte *) (tsv->name), p, strlen (tsv->name));
+  p += 2 * bin2hex ((gdb_byte *) (tsv.name.data ()), p, tsv.name.length ());
   *p++ = '\0';
   putpkt (rs->buf);
   remote_get_noisy_reply ();

gdb/target-debug.h

@@ -126,6 +126,8 @@
   target_debug_do_print (host_address_to_string (X))
 #define target_debug_print_struct_trace_state_variable_p(X)	\
   target_debug_do_print (host_address_to_string (X))
+#define target_debug_print_const_trace_state_variable_r(X)	\
+  target_debug_do_print (host_address_to_string (&X))
 #define target_debug_print_struct_trace_status_p(X)	\
   target_debug_do_print (host_address_to_string (X))
 #define target_debug_print_struct_breakpoint_p(X)	\

gdb/target-delegates.c

@@ -2687,27 +2687,27 @@ debug_can_download_tracepoint (struct target_ops *self)
 }
 
 static void
-delegate_download_trace_state_variable (struct target_ops *self, struct trace_state_variable *arg1)
+delegate_download_trace_state_variable (struct target_ops *self, const trace_state_variable &arg1)
 {
   self = self->beneath;
   self->to_download_trace_state_variable (self, arg1);
 }
 
 static void
-tdefault_download_trace_state_variable (struct target_ops *self, struct trace_state_variable *arg1)
+tdefault_download_trace_state_variable (struct target_ops *self, const trace_state_variable &arg1)
 {
   tcomplain ();
 }
 
 static void
-debug_download_trace_state_variable (struct target_ops *self, struct trace_state_variable *arg1)
+debug_download_trace_state_variable (struct target_ops *self, const trace_state_variable &arg1)
 {
   fprintf_unfiltered (gdb_stdlog, "-> %s->to_download_trace_state_variable (...)\n", debug_target.to_shortname);
   debug_target.to_download_trace_state_variable (&debug_target, arg1);
   fprintf_unfiltered (gdb_stdlog, "<- %s->to_download_trace_state_variable (", debug_target.to_shortname);
   target_debug_print_struct_target_ops_p (&debug_target);
   fputs_unfiltered (", ", gdb_stdlog);
-  target_debug_print_struct_trace_state_variable_p (arg1);
+  target_debug_print_const_trace_state_variable_r (arg1);
   fputs_unfiltered (")\n", gdb_stdlog);
 }
 

gdb/target.h

@@ -957,7 +957,7 @@ struct target_ops
 
     /* Send full details of a trace state variable to the target.  */
     void (*to_download_trace_state_variable) (struct target_ops *,
-					      struct trace_state_variable *tsv)
+					      const trace_state_variable &tsv)
       TARGET_DEFAULT_NORETURN (tcomplain ());
 
     /* Enable a tracepoint on the target.  */

gdb/tracepoint.c

@@ -114,10 +114,7 @@ void (*deprecated_trace_start_stop_hook) (int start, int from_tty);
    any of these for any reason - API is by name or number only - so it
    works to have a vector of objects.  */
 
-typedef struct trace_state_variable tsv_s;
+static std::vector<trace_state_variable> tvariables;
-DEF_VEC_O(tsv_s);
-
-static VEC(tsv_s) *tvariables;
 
 /* The next integer to assign to a variable.  */
 
@@ -267,12 +264,8 @@ set_traceframe_context (struct frame_info *trace_frame)
 struct trace_state_variable *
 create_trace_state_variable (const char *name)
 {
-  struct trace_state_variable tsv;
+  tvariables.emplace_back (name, next_tsv_number++);
-
+  return &tvariables.back ();
-  memset (&tsv, 0, sizeof (tsv));
-  tsv.name = xstrdup (name);
-  tsv.number = next_tsv_number++;
-  return VEC_safe_push (tsv_s, tvariables, &tsv);
 }
 
 /* Look for a trace state variable of the given name.  */
@@ -280,12 +273,9 @@ create_trace_state_variable (const char *name)
 struct trace_state_variable *
 find_trace_state_variable (const char *name)
 {
-  struct trace_state_variable *tsv;
+  for (trace_state_variable &tsv : tvariables)
-  int ix;
+    if (tsv.name == name)
-
+      return &tsv;
-  for (ix = 0; VEC_iterate (tsv_s, tvariables, ix, tsv); ++ix)
-    if (strcmp (name, tsv->name) == 0)
-      return tsv;
 
   return NULL;
 }
@@ -296,12 +286,9 @@ find_trace_state_variable (const char *name)
 struct trace_state_variable *
 find_trace_state_variable_by_number (int number)
 {
-  struct trace_state_variable *tsv;
+  for (trace_state_variable &tsv : tvariables)
-  int ix;
+    if (tsv.number == number)
-
+      return &tsv;
-  for (ix = 0; VEC_iterate (tsv_s, tvariables, ix, tsv); ++ix)
-    if (tsv->number == number)
-      return tsv;
 
   return NULL;
 }
@@ -309,17 +296,11 @@ find_trace_state_variable_by_number (int number)
 static void
 delete_trace_state_variable (const char *name)
 {
-  struct trace_state_variable *tsv;
+  for (auto it = tvariables.begin (); it != tvariables.end (); it++)
-  int ix;
+    if (it->name == name)
-
-  for (ix = 0; VEC_iterate (tsv_s, tvariables, ix, tsv); ++ix)
-    if (strcmp (name, tsv->name) == 0)
       {
-	gdb::observers::tsv_deleted.notify (tsv);
+	gdb::observers::tsv_deleted.notify (&*it);
-
+	tvariables.erase (it);
-	xfree ((void *)tsv->name);
-	VEC_unordered_remove (tsv_s, tvariables, ix);
-
 	return;
       }
 
@@ -394,7 +375,7 @@ trace_variable_command (const char *args, int from_tty)
 	}
       printf_filtered (_("Trace state variable $%s "
 			 "now has initial value %s.\n"),
-		       tsv->name, plongest (tsv->initial_value));
+		       tsv->name.c_str (), plongest (tsv->initial_value));
       return;
     }
 
@@ -406,7 +387,7 @@ trace_variable_command (const char *args, int from_tty)
 
   printf_filtered (_("Trace state variable $%s "
 		     "created, with initial value %s.\n"),
-		   tsv->name, plongest (tsv->initial_value));
+		   tsv->name.c_str (), plongest (tsv->initial_value));
 }
 
 static void
@@ -415,7 +396,7 @@ delete_trace_variable_command (const char *args, int from_tty)
   if (args == NULL)
     {
       if (query (_("Delete all trace state variables? ")))
-	VEC_free (tsv_s, tvariables);
+	tvariables.clear ();
       dont_repeat ();
       gdb::observers::tsv_deleted.notify (NULL);
       return;
@@ -437,41 +418,38 @@ delete_trace_variable_command (const char *args, int from_tty)
 void
 tvariables_info_1 (void)
 {
-  struct trace_state_variable *tsv;
-  int ix;
-  int count = 0;
   struct ui_out *uiout = current_uiout;
 
-  if (VEC_length (tsv_s, tvariables) == 0 && !uiout->is_mi_like_p ())
+  if (tvariables.empty () && !uiout->is_mi_like_p ())
     {
       printf_filtered (_("No trace state variables.\n"));
       return;
     }
 
   /* Try to acquire values from the target.  */
-  for (ix = 0; VEC_iterate (tsv_s, tvariables, ix, tsv); ++ix, ++count)
+  for (trace_state_variable &tsv : tvariables)
-    tsv->value_known = target_get_trace_state_variable_value (tsv->number,
+    tsv.value_known
-							      &(tsv->value));
+      = target_get_trace_state_variable_value (tsv.number, &tsv.value);
 
-  ui_out_emit_table table_emitter (uiout, 3, count, "trace-variables");
+  ui_out_emit_table table_emitter (uiout, 3, tvariables.size (),
+				   "trace-variables");
   uiout->table_header (15, ui_left, "name", "Name");
   uiout->table_header (11, ui_left, "initial", "Initial");
   uiout->table_header (11, ui_left, "current", "Current");
 
   uiout->table_body ();
 
-  for (ix = 0; VEC_iterate (tsv_s, tvariables, ix, tsv); ++ix)
+  for (const trace_state_variable &tsv : tvariables)
     {
       const char *c;
 
       ui_out_emit_tuple tuple_emitter (uiout, "variable");
 
-      std::string name = std::string ("$") + tsv->name;
+      uiout->field_string ("name", std::string ("$") + tsv.name);
-      uiout->field_string ("name", name.c_str ());
+      uiout->field_string ("initial", plongest (tsv.initial_value));
-      uiout->field_string ("initial", plongest (tsv->initial_value));
 
-      if (tsv->value_known)
+      if (tsv.value_known)
-        c = plongest (tsv->value);
+        c = plongest (tsv.value);
       else if (uiout->is_mi_like_p ())
         /* For MI, we prefer not to use magic string constants, but rather
            omit the field completely.  The difference between unknown and
@@ -502,14 +480,11 @@ info_tvariables_command (const char *args, int from_tty)
 void
 save_trace_state_variables (struct ui_file *fp)
 {
-  struct trace_state_variable *tsv;
+  for (const trace_state_variable &tsv : tvariables)
-  int ix;
-
-  for (ix = 0; VEC_iterate (tsv_s, tvariables, ix, tsv); ++ix)
     {
-      fprintf_unfiltered (fp, "tvariable $%s", tsv->name);
+      fprintf_unfiltered (fp, "tvariable $%s", tsv.name.c_str ());
-      if (tsv->initial_value)
+      if (tsv.initial_value)
-	fprintf_unfiltered (fp, " = %s", plongest (tsv->initial_value));
+	fprintf_unfiltered (fp, " = %s", plongest (tsv.initial_value));
       fprintf_unfiltered (fp, "\n");
     }
 }
@@ -1664,10 +1639,8 @@ start_tracing (const char *notes)
   VEC_free (breakpoint_p, tp_vec);
 
   /* Send down all the trace state variables too.  */
-  for (ix = 0; VEC_iterate (tsv_s, tvariables, ix, tsv); ++ix)
+  for (const trace_state_variable &tsv : tvariables)
-    {
+    target_download_trace_state_variable (tsv);
-      target_download_trace_state_variable (tsv);
-    }
   
   /* Tell target to treat text-like sections as transparent.  */
   target_trace_set_readonly_regions ();
@@ -3268,8 +3241,8 @@ merge_uploaded_trace_state_variables (struct uploaded_tsv **uploaded_tsvs)
 
   /* Most likely some numbers will have to be reassigned as part of
      the merge, so clear them all in anticipation.  */
-  for (ix = 0; VEC_iterate (tsv_s, tvariables, ix, tsv); ++ix)
+  for (trace_state_variable &tsv : tvariables)
-    tsv->number = 0;
+    tsv.number = 0;
 
   for (utsv = *uploaded_tsvs; utsv; utsv = utsv->next)
     {
@@ -3279,7 +3252,7 @@ merge_uploaded_trace_state_variables (struct uploaded_tsv **uploaded_tsvs)
 	  if (info_verbose)
 	    printf_filtered (_("Assuming trace state variable $%s "
 			       "is same as target's variable %d.\n"),
-			     tsv->name, utsv->number);
+			     tsv->name.c_str (), utsv->number);
 	}
       else
 	{
@@ -3287,7 +3260,7 @@ merge_uploaded_trace_state_variables (struct uploaded_tsv **uploaded_tsvs)
 	  if (info_verbose)
 	    printf_filtered (_("Created trace state variable "
 			       "$%s for target's variable %d.\n"),
-			     tsv->name, utsv->number);
+			     tsv->name.c_str (), utsv->number);
 	}
       /* Give precedence to numberings that come from the target.  */
       if (tsv)
@@ -3296,14 +3269,13 @@ merge_uploaded_trace_state_variables (struct uploaded_tsv **uploaded_tsvs)
 
   /* Renumber everything that didn't get a target-assigned number.  */
   highest = 0;
-  for (ix = 0; VEC_iterate (tsv_s, tvariables, ix, tsv); ++ix)
+  for (const trace_state_variable &tsv : tvariables)
-    if (tsv->number > highest)
+    highest = std::max (tsv.number, highest);
-      highest = tsv->number;
 
   ++highest;
-  for (ix = 0; VEC_iterate (tsv_s, tvariables, ix, tsv); ++ix)
+  for (trace_state_variable &tsv : tvariables)
-    if (tsv->number == 0)
+    if (tsv.number == 0)
-      tsv->number = highest++;
+      tsv.number = highest++;
 
   free_uploaded_tsvs (uploaded_tsvs);
 }

gdb/tracepoint.h

@@ -47,29 +47,33 @@ typedef std::unique_ptr<traceframe_info> traceframe_info_up;
    tracepoints.  */
 
 struct trace_state_variable
-  {
+{
-    /* The variable's name.  The user has to prefix with a dollar sign,
+  trace_state_variable (std::string &&name_, int number_)
-       but we don't store that internally.  */
+  : name (name_), number (number_)
-    const char *name;
+  {}
 
-    /* An id number assigned by GDB, and transmitted to targets.  */
+  /* The variable's name.  The user has to prefix with a dollar sign,
-    int number;
+     but we don't store that internally.  */
+  std::string name;
 
-    /* The initial value of a variable is a 64-bit signed integer.  */
+  /* An id number assigned by GDB, and transmitted to targets.  */
-    LONGEST initial_value;
+  int number = 0;
 
-    /* 1 if the value is known, else 0.  The value is known during a
+  /* The initial value of a variable is a 64-bit signed integer.  */
-       trace run, or in tfind mode if the variable was collected into
+  LONGEST initial_value = 0;
-       the current trace frame.  */
-    int value_known;
 
-    /* The value of a variable is a 64-bit signed integer.  */
+  /* 1 if the value is known, else 0.  The value is known during a
-    LONGEST value;
+     trace run, or in tfind mode if the variable was collected into
+     the current trace frame.  */
+  int value_known = 0;
 
-    /* This is true for variables that are predefined and built into
+  /* The value of a variable is a 64-bit signed integer.  */
-       the target.  */
+  LONGEST value = 0;
-    int builtin;
+
-   };
+  /* This is true for variables that are predefined and built into
+     the target.  */
+  int builtin = 0;
+ };
 
 /* The trace status encompasses various info about the general state
    of the tracing run.  */