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

[Pedro Alves <palves@redhat.com>]

On 06/25/2018 09:51 PM, Pedro Franco de Carvalho wrote:
> Ulrich Weigand <uweigand@de.ibm.com> writes:
> 
>> 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).
> 
> I don't think a partial agent expression would be sent in this case,
> since this is before the first putpkt is called in the function. But I
> can still issue the warning and ignore the condition expression instead
> of failing on the assertion. Otherwise I can check the size once and
> call a gdb_assert if its too small, like the rest of the function. Which
> is better?

I'm not sure I understand the details or the suggestions below (the
patches don't seem to be meant to apply on top of current master, but
they're using buf.data()), but I'd just like to point out that ideally
GDB should not gdb_assert or abort on user input or remote stub
limitations (small remote packet size), since neither are a GDB bug.

> 
> Would something like one of the two alternative below be ok for checking
> the size only once?
> 
> The second one looks complicated, but my goal was to avoid overflows in
> 2 * aexpr->len, since that length ultimately comes from the condition
> expression the user supplies.
> 
> I am also assuming throughout this function that size_t and
> gdb::char_vector::size_type are compatible (since buf.size () returns
> the latter and xsnprintf takes a size_t). Is this ok?

It is.

Thanks,
Pedro Alves