Re: [PATCH 3/4] Use get_remote_packet_size in download_tracepoint

Ulrich Weigand
2018-06-25T10:37:28Z

Pedro Franco de Carvalho wrote:

> * 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.

This makes sense in general, but I'm not sure it makes sense to use
a separate hex_byte buffer here:

> {
> 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);
> + }
> }

You know from the beginning that the agent expression will take
(2 * aexpr->len) bytes, so it should be OK to only check this
once, ahead of time. In fact, sending a partial agent expression
seems to be worse than sending none, so if the agent expression
is too long, I think it should be just omitted (and the user
warned).

Bye,
Ulrich

--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com