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

["Ulrich Weigand" <uweigand@de.ibm.com>]

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