Re: [PATCH v5 2/4] target/arm: Change GDBState's line_buf to a GString

[Peter Maydell <peter.maydell@linaro.org>]

On Mon, 11 Aug 2025 at 20:37, Vacha Bhavsar
<vacha.bhavsar@oss.qualcomm.com> wrote:
>
> This patch changes GDBState's line_buf from a character
> array to a GString. This allows line_buf to be dynamically
> re-sizeable.
>
> Signed-off-by: Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com>
> ---
> Changes since v4:
> - this patch was not present in v4, it has been added as
> suggested during review of v4
> ---

Note that since the GDBState is a file-local variable,
not allocated on the stack, it's not an issue that
line_buf[MAX_PACKET_LENGTH] is large. So whether we
make this change I think should be based on whether
it makes the code easier to read and less likely to
contain string buffer manipulation bugs.


> -                if (gdbserver_state.line_buf_index + repeat >= > sizeof(gdbserver_state.line_buf) - 1) {
> +                if (gdbserver_state.line_buf->len + repeat >= MAX_PACKET_LENGTH) {
>                      /* that many repeats would overrun the command buffer */

This comment no longer makes sense if we don't have a
fixed command buffer. In general, if we're moving away
from a fixed-sized buffer we should consider what we
want to do in all the various places that are currently
doing checks against MAX_PACKET_LENGTH.

thanks
-- PMM