[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/2] gdbstub: do not split gdb_monitor_write payload
From: |
Alex Bennée |
Subject: |
Re: [PATCH v2 2/2] gdbstub: do not split gdb_monitor_write payload |
Date: |
Thu, 12 Dec 2019 10:52:53 +0000 |
User-agent: |
mu4e 1.3.5; emacs 27.0.50 |
Damien Hedde <address@hidden> writes:
> On 12/11/19 7:59 PM, Alex Bennée wrote:
>>
>> Damien Hedde <address@hidden> writes:
>>
>>> Since we can now send packets of arbitrary length:
>>> simplify gdb_monitor_write() and send the whole payload
>>> in one packet.
>>
>> Do we know gdb won't barf on us. Does the negotiated max packet size
>> only apply to data sent to the gdbserver?
>
> Yes the negociated packet size is only about packet we can receive.
> Qutoting the gdb doc:
> | ‘PacketSize=bytes’
> |
> | The remote stub can accept packets up to at least bytes in length.
> | GDB will send packets up to this size for bulk transfers, and will
> | never send larger packets.
>
> The qSupported doc also says that "Any GDB which sends a ‘qSupported’
> packet supports receiving packets of unlimited length".
> I did some digging and qSupported appeared in gdb 6.6 (december 2006).
> And gdb supported arbitrary sized packet even before that (6.4.9 2006
> too).
I think that is worth a comment for the function gdb_monitor_write
quoting the spec and the versions. With that comment:
Reviewed-by: Alex Bennée <address@hidden>
>
>>
>>>
>>> Suggested-by: Luc Michel <address@hidden>
>>> Signed-off-by: Damien Hedde <address@hidden>
>>> ---
>>> gdbstub.c | 23 +++--------------------
>>> 1 file changed, 3 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/gdbstub.c b/gdbstub.c
>>> index 93b26f1b86..ef999abee2 100644
>>> --- a/gdbstub.c
>>> +++ b/gdbstub.c
>>> @@ -3200,28 +3200,11 @@ static void gdb_chr_event(void *opaque, int event)
>>> }
>>> }
>>>
>>> -static void gdb_monitor_output(GDBState *s, const char *msg, int len)
>>> -{
>>> - g_autoptr(GString) buf = g_string_new("O");
>>> - memtohex(buf, (uint8_t *)msg, len);
>>> - put_packet(buf->str);
>>> -}
>>> -
>>> static int gdb_monitor_write(Chardev *chr, const uint8_t *buf, int len)
>>> {
>>> - const char *p = (const char *)buf;
>>> - int max_sz;
>>> -
>>> - max_sz = (MAX_PACKET_LENGTH / 2) + 1;
>>> - for (;;) {
>>> - if (len <= max_sz) {
>>> - gdb_monitor_output(&gdbserver_state, p, len);
>>> - break;
>>> - }
>>> - gdb_monitor_output(&gdbserver_state, p, max_sz);
>>> - p += max_sz;
>>> - len -= max_sz;
>>> - }
>>> + g_autoptr(GString) hex_buf = g_string_new("O");
>>> + memtohex(hex_buf, buf, len);
>>> + put_packet(hex_buf->str);
>>> return len;
>>> }
>>
>>
--
Alex Bennée
[PATCH v2 1/2] gdbstub: change GDBState.last_packet to GByteArray, Damien Hedde, 2019/12/11