[PATCH 3/4] Use get_remote_packet_size in download_tracepoint

Pedro Franco de Carvalho
2018-06-20T21:09:26Z

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