[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 04/20] gdbstub: move mem_buf to GDBState and use GByteArra
From: |
Alex Bennée |
Subject: |
Re: [PATCH v3 04/20] gdbstub: move mem_buf to GDBState and use GByteArray |
Date: |
Thu, 19 Dec 2019 14:44:24 +0000 |
User-agent: |
mu4e 1.3.5; emacs 27.0.50 |
Damien Hedde <address@hidden> writes:
> 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.
Oops, fixed. I should assert len is 0 before we start.
>
>> + 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
--
Alex Bennée
- [PATCH v3 00/20] gdbstub refactor and SVE support (+check-tcg tweaks), Alex Bennée, 2019/12/11
- [PATCH v3 05/20] gdbstub: add helper for 128 bit registers, Alex Bennée, 2019/12/11
- [PATCH v3 01/20] gdbstub: make GDBState static and have common init function, Alex Bennée, 2019/12/11
- [PATCH v3 04/20] gdbstub: move mem_buf to GDBState and use GByteArray, Alex Bennée, 2019/12/11
- [PATCH v3 06/20] target/arm: use gdb_get_reg helpers, Alex Bennée, 2019/12/11
- [PATCH v3 03/20] gdbstub: move str_buf to GDBState and use GString, Alex Bennée, 2019/12/11
- [PATCH v3 07/20] target/m68k: use gdb_get_reg helpers, Alex Bennée, 2019/12/11
- [PATCH v3 10/20] target/arm: explicitly encode regnum in our XML, Alex Bennée, 2019/12/11
- [PATCH v3 11/20] target/arm: default SVE length to 64 bytes for linux-user, Alex Bennée, 2019/12/11
- [PATCH v3 09/20] target/arm: prepare for multiple dynamic XMLs, Alex Bennée, 2019/12/11
- [PATCH v3 19/20] tests/tcg/aarch64: add SVE iotcl test, Alex Bennée, 2019/12/11