Re: [PATCH 1/1] gdb, gdbserver: replace PBUFSIZ with a target op

[Pedro Alves <pedro@palves.net>]

Hi!

On 2023-09-20 13:59, Andrew Burgess via Gdb-patches wrote:
> "Gerlicher, Klaus via Gdb-patches" <gdb-patches@sourceware.org> writes:
> 
>> Hi Simon,
>>
>> Thanks for the quick response.
>>
>> At least the initial buffer size needs to be fixed since now most
>> clients aren't aware of any dynamic behavior here and therefore we
>> need at least something pre-allocated for these clients.
> 
> I don't understand your concerns here.  For this patch we're only
> talking about the gdbserver client, right?  And your patch (rightly)
> doesn't change things on the GDB side.

The client is the GDB side, the server side is, well, gdbserver.  :-)

> 
> GDB already uses a dynamic packet buffer size.  

It is dynamic, but not in the sense that we just append to the buffer
with push_back and let the buffer grow unbounded.  Instead, GDB tries to guess a
sufficient packet size, but if the server tells it explicitly what packet size it
supports, then GDB will grow its buffer to that size, no questions asked.

  /* If we increased the packet size, make sure to increase the global
     buffer size also.  We delay this until after parsing the entire
     qSupported packet, because this is the same buffer we were
     parsing.  */
  if (rs->buf.size () < rs->explicit_packet_size)
    rs->buf.resize (rs->explicit_packet_size);


> So the only initial
> buffer I think you can be talking about here is the gdbserver buffer,
> which I think could be made dynamic, just as GDB's is.

I think he was really talking about the GDB side.  Or even other
clients, like LLDB, etc.

> 
> We could hard-code gdbserver to return some stupidly large number for
> the PacketSize in the qSupported reply, say MAX_INT?  Or (MAX_INT / 4),
> you pick, this could be anything really, just something huge.

I don't think it can, due to the immediate resize mentioned above.

I don't understand why the patch added a target method on the gdb side.

Also, do we really need the new target method on the gdbserver side?
We assert that the buffer size is bigger than the tdesc's register size plus
a slack, but how about flipping that around and make the buffer size be dependent
on the register size?  Maybe the packet size decision is done earlier than we
know which tdesc we are using, though, that's something to check.

Pedro Alves