Re: [PATCH v3 04/20] gdbstub: move mem_buf to GDBState and use GByteArray

[Damien Hedde]

On 12/11/19 6:05 PM, Alex Bennée wrote:
> This is in preparation for further re-factoring of the register API
> with the rest of the code. Theoretically the read register function
> could overwrite the MAX_PACKET_LENGTH buffer although currently all
> registers are well within the size range.
> 
> Signed-off-by: Alex Bennée <address@hidden>
> Reviewed-by: Richard Henderson <address@hidden>
> Reviewed-by: Damien Hedde <address@hidden>
> Tested-by: Damien Hedde <address@hidden>
> 
> ---
> v3
>   - fixed up email on Damien's tags
> ---
>  gdbstub.c | 56 ++++++++++++++++++++++++++++++++++---------------------
>  1 file changed, 35 insertions(+), 21 deletions(-)
> 

> @@ -2092,11 +2105,12 @@ static void handle_query_rcmd(GdbCmdContext *gdb_ctx, 
> void *user_ctx)
>      }
>  
>      len = len / 2;
> -    hextomem(gdb_ctx->mem_buf, gdb_ctx->params[0].data, len);
> -    gdb_ctx->mem_buf[len++] = 0;
> -    qemu_chr_be_write(gdbserver_state.mon_chr, gdb_ctx->mem_buf, len);
> +    g_byte_array_set_size(gdbserver_state.mem_buf, len);

Hi Alex,

Just found out that the g_byte_array_set_size() above should be removed.
hextomem() will append data starting at offset [len] instead of [0] and
we end up with an uninitialized prefix in the array.

> +    hextomem(gdbserver_state.mem_buf, gdb_ctx->params[0].data, len);
> +    g_byte_array_append(gdbserver_state.mem_buf, &zero, 1);
> +    qemu_chr_be_write(gdbserver_state.mon_chr, gdbserver_state.mem_buf->data,
> +                      gdbserver_state.mem_buf->len);
>      put_packet("OK");
> -
>  }
>  #endif
>  
> 

I did double-checked the rest of the patch and it is it the only resize
that passed through v2 review.

Regards,
Damien