[binutils-gdb] Make encode_actions_rsp use std::vector

[Simon Marchi <simark at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=b44ec61915f10a953ca85da5bf7a97911554f1aa

commit b44ec61915f10a953ca85da5bf7a97911554f1aa
Author: Simon Marchi <simon.marchi@ericsson.com>
Date:   Wed Nov 8 19:42:08 2017 -0500

    Make encode_actions_rsp use std::vector
    
    Currently, encode_actions_rsp returns two malloc'ed arrays of malloc'ed
    strings (char *) by pointer.  Change this to use
    std::vector<std::string>.  This eliminates some cleanups in remote.c.
    
    Regtested on the buildbot.
    
    gdb/ChangeLog:
    
    	* tracepoint.h (class collection_list) <stringify>: Return
    	std::vector<std::string>.
    	(encode_actions_rsp): Change parameters to
    	std::vector<std::string> *.
    	* tracepoint.c (collection_list::stringify): Return
    	std::vector<std::string> and adjust accordingly.
    	(encode_actions_rsp): Changee parameters to
    	std::vector<std::string> and adjust accordingly.
    	* remote.c (free_actions_list),
    	free_actions_list_cleanup_wrapper): Remove.
    	(remote_download_tracepoint): Adjust to std::vector.

Diff:
---
 gdb/ChangeLog    |  14 ++++++++
 gdb/remote.c     | 100 ++++++++++++++++++++-----------------------------------
 gdb/tracepoint.c |  40 ++++++----------------
 gdb/tracepoint.h |   5 +--
 4 files changed, 65 insertions(+), 94 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index cf9ccfc..2be36de 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,17 @@
+2017-11-08  Simon Marchi  <simon.marchi@ericsson.com>
+
+	* tracepoint.h (class collection_list) <stringify>: Return
+	std::vector<std::string>.
+	(encode_actions_rsp): Change parameters to
+	std::vector<std::string> *.
+	* tracepoint.c (collection_list::stringify): Return
+	std::vector<std::string> and adjust accordingly.
+	(encode_actions_rsp): Changee parameters to
+	std::vector<std::string> and adjust accordingly.
+	* remote.c (free_actions_list),
+	free_actions_list_cleanup_wrapper): Remove.
+	(remote_download_tracepoint): Adjust to std::vector.
+
 2017-11-08  Tom Tromey  <tom@tromey.com>
 
 	* dwarf2read.c (symbolp): Remove typedef.
diff --git a/gdb/remote.c b/gdb/remote.c
index 3bf0596..c653562 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -12289,28 +12289,6 @@ remote_trace_init (struct target_ops *self)
     error (_("Target does not support this command."));
 }
 
-static void free_actions_list (char **actions_list);
-static void free_actions_list_cleanup_wrapper (void *);
-static void
-free_actions_list_cleanup_wrapper (void *al)
-{
-  free_actions_list ((char **) al);
-}
-
-static void
-free_actions_list (char **actions_list)
-{
-  int ndx;
-
-  if (actions_list == 0)
-    return;
-
-  for (ndx = 0; actions_list[ndx]; ndx++)
-    xfree (actions_list[ndx]);
-
-  xfree (actions_list);
-}
-
 /* Recursive routine to walk through command list including loops, and
    download packets for each command.  */
 
@@ -12359,20 +12337,14 @@ remote_download_tracepoint (struct target_ops *self, struct bp_location *loc)
   CORE_ADDR tpaddr;
   char addrbuf[40];
   char buf[BUF_SIZE];
-  char **tdp_actions;
-  char **stepping_actions;
-  int ndx;
-  struct cleanup *old_chain = NULL;
+  std::vector<std::string> tdp_actions;
+  std::vector<std::string> stepping_actions;
   char *pkt;
   struct breakpoint *b = loc->owner;
   struct tracepoint *t = (struct tracepoint *) b;
   struct remote_state *rs = get_remote_state ();
 
   encode_actions_rsp (loc, &tdp_actions, &stepping_actions);
-  old_chain = make_cleanup (free_actions_list_cleanup_wrapper,
-			    tdp_actions);
-  (void) make_cleanup (free_actions_list_cleanup_wrapper,
-		       stepping_actions);
 
   tpaddr = loc->address;
   sprintf_vma (addrbuf, tpaddr);
@@ -12438,7 +12410,7 @@ remote_download_tracepoint (struct target_ops *self, struct bp_location *loc)
 	  xsnprintf (buf + strlen (buf), BUF_SIZE - strlen (buf), ":X%x,",
 		     aexpr->len);
 	  pkt = buf + strlen (buf);
-	  for (ndx = 0; ndx < aexpr->len; ++ndx)
+	  for (int ndx = 0; ndx < aexpr->len; ++ndx)
 	    pkt = pack_hex_byte (pkt, aexpr->buf[ndx]);
 	  *pkt = '\0';
 	}
@@ -12455,39 +12427,43 @@ remote_download_tracepoint (struct target_ops *self, struct bp_location *loc)
     error (_("Target does not support tracepoints."));
 
   /* do_single_steps (t); */
-  if (tdp_actions)
+  for (auto action_it = tdp_actions.begin ();
+       action_it != tdp_actions.end (); action_it++)
     {
-      for (ndx = 0; tdp_actions[ndx]; ndx++)
-	{
-	  QUIT;	/* Allow user to bail out with ^C.  */
-	  xsnprintf (buf, BUF_SIZE, "QTDP:-%x:%s:%s%c",
-		     b->number, addrbuf, /* address */
-		     tdp_actions[ndx],
-		     ((tdp_actions[ndx + 1] || stepping_actions)
-		      ? '-' : 0));
-	  putpkt (buf);
-	  remote_get_noisy_reply ();
-	  if (strcmp (rs->buf, "OK"))
-	    error (_("Error on target while setting tracepoints."));
-	}
-    }
-  if (stepping_actions)
-    {
-      for (ndx = 0; stepping_actions[ndx]; ndx++)
-	{
-	  QUIT;	/* Allow user to bail out with ^C.  */
-	  xsnprintf (buf, BUF_SIZE, "QTDP:-%x:%s:%s%s%s",
-		     b->number, addrbuf, /* address */
-		     ((ndx == 0) ? "S" : ""),
-		     stepping_actions[ndx],
-		     (stepping_actions[ndx + 1] ? "-" : ""));
-	  putpkt (buf);
-	  remote_get_noisy_reply ();
-	  if (strcmp (rs->buf, "OK"))
-	    error (_("Error on target while setting tracepoints."));
-	}
+      QUIT;	/* Allow user to bail out with ^C.  */
+
+      bool has_more = (action_it != tdp_actions.end ()
+		       || !stepping_actions.empty ());
+
+      xsnprintf (buf, BUF_SIZE, "QTDP:-%x:%s:%s%c",
+		 b->number, addrbuf, /* address */
+		 action_it->c_str (),
+		 has_more ? '-' : 0);
+      putpkt (buf);
+      remote_get_noisy_reply ();
+      if (strcmp (rs->buf, "OK"))
+	error (_("Error on target while setting tracepoints."));
     }
 
+    for (auto action_it = stepping_actions.begin ();
+	 action_it != stepping_actions.end (); action_it++)
+      {
+	QUIT;	/* Allow user to bail out with ^C.  */
+
+	bool is_first = action_it == stepping_actions.begin ();
+	bool has_more = action_it != stepping_actions.end ();
+
+	xsnprintf (buf, BUF_SIZE, "QTDP:-%x:%s:%s%s%s",
+		   b->number, addrbuf, /* address */
+		   is_first ? "S" : "",
+		   action_it->c_str (),
+		   has_more ? "-" : "");
+	putpkt (buf);
+	remote_get_noisy_reply ();
+	if (strcmp (rs->buf, "OK"))
+	  error (_("Error on target while setting tracepoints."));
+      }
+
   if (packet_support (PACKET_TracepointSource) == PACKET_ENABLE)
     {
       if (b->location != NULL)
@@ -12515,8 +12491,6 @@ remote_download_tracepoint (struct target_ops *self, struct bp_location *loc)
       remote_download_command_source (b->number, loc->address,
 				      breakpoint_commands (b));
     }
-
-  do_cleanups (old_chain);
 }
 
 static int
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 134695e..59a7b64 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1118,18 +1118,14 @@ collection_list::collection_list ()
 
 /* Reduce a collection list to string form (for gdb protocol).  */
 
-char **
+std::vector<std::string>
 collection_list::stringify ()
 {
   char temp_buf[2048];
   int count;
-  int ndx = 0;
-  char *(*str_list)[];
   char *end;
   long i;
-
-  count = 1 + 1 + m_memranges.size () + m_aexprs.size () + 1;
-  str_list = (char *(*)[]) xmalloc (count * sizeof (char *));
+  std::vector<std::string> str_list;
 
   if (m_strace_data)
     {
@@ -1137,8 +1133,7 @@ collection_list::stringify ()
 	printf_filtered ("\nCollecting static trace data\n");
       end = temp_buf;
       *end++ = 'L';
-      (*str_list)[ndx] = savestring (temp_buf, end - temp_buf);
-      ndx++;
+      str_list.emplace_back (temp_buf, end - temp_buf);
     }
 
   for (i = sizeof (m_regs_mask) - 1; i > 0; i--)
@@ -1158,8 +1153,7 @@ collection_list::stringify ()
 	  sprintf (end, "%02X", m_regs_mask[i]);
 	  end += 2;
 	}
-      (*str_list)[ndx] = xstrdup (temp_buf);
-      ndx++;
+      str_list.emplace_back (temp_buf);
     }
   if (info_verbose)
     printf_filtered ("\n");
@@ -1179,8 +1173,7 @@ collection_list::stringify ()
 	}
       if (count + 27 > MAX_AGENT_EXPR_LEN)
 	{
-	  (*str_list)[ndx] = savestring (temp_buf, count);
-	  ndx++;
+	  str_list.emplace_back (temp_buf, count);
 	  count = 0;
 	  end = temp_buf;
 	}
@@ -1210,8 +1203,7 @@ collection_list::stringify ()
       QUIT;			/* Allow user to bail out with ^C.  */
       if ((count + 10 + 2 * m_aexprs[i]->len) > MAX_AGENT_EXPR_LEN)
 	{
-	  (*str_list)[ndx] = savestring (temp_buf, count);
-	  ndx++;
+	  str_list.emplace_back (temp_buf, count);
 	  count = 0;
 	  end = temp_buf;
 	}
@@ -1225,20 +1217,12 @@ collection_list::stringify ()
 
   if (count != 0)
     {
-      (*str_list)[ndx] = savestring (temp_buf, count);
-      ndx++;
+      str_list.emplace_back (temp_buf, count);
       count = 0;
       end = temp_buf;
     }
-  (*str_list)[ndx] = NULL;
 
-  if (ndx == 0)
-    {
-      xfree (str_list);
-      return NULL;
-    }
-  else
-    return *str_list;
+  return str_list;
 }
 
 /* Add the printed expression EXP to *LIST.  */
@@ -1513,14 +1497,12 @@ encode_actions (struct bp_location *tloc,
 /* Render all actions into gdb protocol.  */
 
 void
-encode_actions_rsp (struct bp_location *tloc, char ***tdp_actions,
-		    char ***stepping_actions)
+encode_actions_rsp (struct bp_location *tloc,
+		    std::vector<std::string> *tdp_actions,
+		    std::vector<std::string> *stepping_actions)
 {
   struct collection_list tracepoint_list, stepping_list;
 
-  *tdp_actions = NULL;
-  *stepping_actions = NULL;
-
   encode_actions (tloc, &tracepoint_list, &stepping_list);
 
   *tdp_actions = tracepoint_list.stringify ();
diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
index 18fdcf3..60caa7a 100644
--- a/gdb/tracepoint.h
+++ b/gdb/tracepoint.h
@@ -269,7 +269,7 @@ public:
 
   void finish ();
 
-  char **stringify ();
+  std::vector<std::string> stringify ();
 
   const std::vector<std::string> &wholly_collected ()
   { return m_wholly_collected; }
@@ -328,7 +328,8 @@ extern void encode_actions (struct bp_location *tloc,
 			    struct collection_list *stepping_list);
 
 extern void encode_actions_rsp (struct bp_location *tloc,
-				char ***tdp_actions, char ***stepping_actions);
+				std::vector<std::string> *tdp_actions,
+				std::vector<std::string> *stepping_actions);
 
 extern void validate_actionline (const char *, struct breakpoint *);
 extern void validate_trace_state_variable_name (const char *name);