[PATCH 3/4] Use get_remote_packet_size in download_tracepoint

Pedro Franco de Carvalho pedromfc@linux.vnet.ibm.com
Wed Jun 20 21:09:00 GMT 2018


This patch changes the remote target to use the remote packet size to
build QTDP packets.

The char array used to build the packets is changed to a
gdb::char_vector and sized with the result from
get_remote_packet_size. All writes to the buffer are changed to use
xsnprintf, so that no assumptions on the size of the action strings
are needed.

gdb/ChangeLog:
YYYY-MM-DD  Pedro Franco de Carvalho  <pedromfc@linux.vnet.ibm.com>

	* remote.c (remote_target::download_tracepoint): Remove BUF_SIZE
	and pkt. Replace array buf with gdb::char_vector buf, of size
	get_remote_packet_size (). Replace references to buf and BUF_SIZE
	to buf.data () and buf.size (). Replace strcpy, strcat and
	pack_hex_byte with xsnprintf.
---
 gdb/remote.c | 68 ++++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 43 insertions(+), 25 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 04dc67fdc0..f5fbb7e7ce 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -12793,23 +12793,24 @@ remote_target::remote_download_command_source (int num, ULONGEST addr,
 void
 remote_target::download_tracepoint (struct bp_location *loc)
 {
-#define BUF_SIZE 2048
-
   CORE_ADDR tpaddr;
   char addrbuf[40];
-  char buf[BUF_SIZE];
   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 ();
 
+  /* We use a buffer other than rs->buf because we'll build strings
+     across multiple statements, and other statements in between could
+     modify rs->buf.  */
+  gdb::char_vector buf (get_remote_packet_size ());
+
   encode_actions_rsp (loc, &tdp_actions, &stepping_actions);
 
   tpaddr = loc->address;
   sprintf_vma (addrbuf, tpaddr);
-  xsnprintf (buf, BUF_SIZE, "QTDP:%x:%s:%c:%lx:%x", b->number,
+  xsnprintf (buf.data (), buf.size (), "QTDP:%x:%s:%c:%lx:%x", b->number,
 	     addrbuf, /* address */
 	     (b->enable_state == bp_enabled ? 'E' : 'D'),
 	     t->step_count, t->pass_count);
@@ -12824,8 +12825,9 @@ remote_target::download_tracepoint (struct bp_location *loc)
 	{
 	  if (gdbarch_fast_tracepoint_valid_at (loc->gdbarch, tpaddr,
 						NULL))
-	    xsnprintf (buf + strlen (buf), BUF_SIZE - strlen (buf), ":F%x",
-		       gdb_insn_length (loc->gdbarch, tpaddr));
+	    xsnprintf (buf.data () + strlen (buf.data ()),
+		       buf.size () - strlen (buf.data ()),
+		       ":F%x", gdb_insn_length (loc->gdbarch, tpaddr));
 	  else
 	    /* If it passed validation at definition but fails now,
 	       something is very wrong.  */
@@ -12849,7 +12851,8 @@ remote_target::download_tracepoint (struct bp_location *loc)
 	  struct static_tracepoint_marker marker;
 
 	  if (target_static_tracepoint_marker_at (tpaddr, &marker))
-	    strcat (buf, ":S");
+	    xsnprintf (buf.data () + strlen (buf.data ()),
+		       buf.size () - strlen (buf.data ()), ":S");
 	  else
 	    error (_("Static tracepoint not valid during download"));
 	}
@@ -12868,12 +12871,23 @@ remote_target::download_tracepoint (struct bp_location *loc)
       if (remote_supports_cond_tracepoints ())
 	{
 	  agent_expr_up aexpr = gen_eval_for_expr (tpaddr, loc->cond.get ());
-	  xsnprintf (buf + strlen (buf), BUF_SIZE - strlen (buf), ":X%x,",
+	  xsnprintf (buf.data () + strlen (buf.data ()),
+		     buf.size () - strlen (buf.data ()), ":X%x,",
 		     aexpr->len);
-	  pkt = buf + strlen (buf);
+
+	  char *end = buf.data () + strlen (buf.data ());
+	  long size_left = buf.size () - strlen (buf.data ());
+	  char hex_byte[3];
+	  hex_byte[2] = '\0';
+
 	  for (int ndx = 0; ndx < aexpr->len; ++ndx)
-	    pkt = pack_hex_byte (pkt, aexpr->buf[ndx]);
-	  *pkt = '\0';
+	    {
+	      pack_hex_byte (hex_byte, aexpr->buf[ndx]);
+	      xsnprintf (end, size_left, hex_byte);
+
+	      size_left -= strlen (end);
+	      end += strlen (end);
+	    }
 	}
       else
 	warning (_("Target does not support conditional tracepoints, "
@@ -12881,8 +12895,10 @@ remote_target::download_tracepoint (struct bp_location *loc)
     }
 
   if (b->commands || *default_collect)
-    strcat (buf, "-");
-  putpkt (buf);
+    xsnprintf (buf.data () + strlen (buf.data ()),
+	       buf.size () - strlen (buf.data ()), "-");
+
+  putpkt (buf.data ());
   remote_get_noisy_reply ();
   if (strcmp (rs->buf, "OK"))
     error (_("Target does not support tracepoints."));
@@ -12896,11 +12912,11 @@ remote_target::download_tracepoint (struct bp_location *loc)
       bool has_more = ((action_it + 1) != tdp_actions.end ()
 		       || !stepping_actions.empty ());
 
-      xsnprintf (buf, BUF_SIZE, "QTDP:-%x:%s:%s%c",
+      xsnprintf (buf.data (), buf.size (), "QTDP:-%x:%s:%s%c",
 		 b->number, addrbuf, /* address */
 		 action_it->c_str (),
 		 has_more ? '-' : 0);
-      putpkt (buf);
+      putpkt (buf.data ());
       remote_get_noisy_reply ();
       if (strcmp (rs->buf, "OK"))
 	error (_("Error on target while setting tracepoints."));
@@ -12914,12 +12930,12 @@ remote_target::download_tracepoint (struct bp_location *loc)
       bool is_first = action_it == stepping_actions.begin ();
       bool has_more = (action_it + 1) != stepping_actions.end ();
 
-      xsnprintf (buf, BUF_SIZE, "QTDP:-%x:%s:%s%s%s",
+      xsnprintf (buf.data (), buf.size (), "QTDP:-%x:%s:%s%s%s",
 		 b->number, addrbuf, /* address */
 		 is_first ? "S" : "",
 		 action_it->c_str (),
 		 has_more ? "-" : "");
-      putpkt (buf);
+      putpkt (buf.data ());
       remote_get_noisy_reply ();
       if (strcmp (rs->buf, "OK"))
 	error (_("Error on target while setting tracepoints."));
@@ -12929,22 +12945,24 @@ remote_target::download_tracepoint (struct bp_location *loc)
     {
       if (b->location != NULL)
 	{
-	  strcpy (buf, "QTDPsrc:");
+	  xsnprintf (buf.data (), buf.size (), "QTDPsrc:");
 	  encode_source_string (b->number, loc->address, "at",
 				event_location_to_string (b->location.get ()),
-				buf + strlen (buf), 2048 - strlen (buf));
-	  putpkt (buf);
+				buf.data () + strlen (buf.data ()),
+				buf.size () - strlen (buf.data ()));
+	  putpkt (buf.data ());
 	  remote_get_noisy_reply ();
 	  if (strcmp (rs->buf, "OK"))
 	    warning (_("Target does not support source download."));
 	}
       if (b->cond_string)
 	{
-	  strcpy (buf, "QTDPsrc:");
+	  xsnprintf (buf.data (), buf.size (), "QTDPsrc:");
 	  encode_source_string (b->number, loc->address,
-				"cond", b->cond_string, buf + strlen (buf),
-				2048 - strlen (buf));
-	  putpkt (buf);
+				"cond", b->cond_string,
+				buf.data () + strlen (buf.data ()),
+				buf.size () - strlen (buf.data ()));
+	  putpkt (buf.data ());
 	  remote_get_noisy_reply ();
 	  if (strcmp (rs->buf, "OK"))
 	    warning (_("Target does not support source download."));
-- 
2.13.6



More information about the Gdb-patches mailing list