Since we can now send packets of arbitrary length:
simplify gdb_monitor_write() and send the whole payload
in one packet.
Suggested-by: Luc Michel <luc.michel@greensocs.com>
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
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;
}
--
2.24.0
Damien Hedde <damien.hedde@greensocs.com> 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?
>
> Suggested-by: Luc Michel <luc.michel@greensocs.com>
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> ---
> 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
On 12/11/19 7:59 PM, Alex Bennée wrote:
>
> Damien Hedde <damien.hedde@greensocs.com> 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).
>
>>
>> Suggested-by: Luc Michel <luc.michel@greensocs.com>
>> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
>> ---
>> 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;
>> }
>
>
Damien Hedde <damien.hedde@greensocs.com> writes:
> On 12/11/19 7:59 PM, Alex Bennée wrote:
>>
>> Damien Hedde <damien.hedde@greensocs.com> 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 <alex.bennee@linaro.org>
>
>>
>>>
>>> Suggested-by: Luc Michel <luc.michel@greensocs.com>
>>> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
>>> ---
>>> 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
On 12/12/19 11:52 AM, Alex Bennée wrote: > > Damien Hedde <damien.hedde@greensocs.com> writes: > >> On 12/11/19 7:59 PM, Alex Bennée wrote: >>> >>> Damien Hedde <damien.hedde@greensocs.com> 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 <alex.bennee@linaro.org> > Good idea ! Is that ok if I add these comments in the 1st patch along with the gdbstate.last_packet field ? it seems a more central place. I can still add a short note for gdb_monitor_write(). Damien
On 12/11/19 5:05 PM, Damien Hedde wrote:
> Since we can now send packets of arbitrary length:
> simplify gdb_monitor_write() and send the whole payload
> in one packet.
While we can send arbitrary length on the wire, we advertise
PacketSize=MAX_PACKET_LENGTH in handle_query_supported().
We can raise this limit however.
> Suggested-by: Luc Michel <luc.michel@greensocs.com>
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> ---
> 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;
> }
>
>
On 12/11/19 7:58 PM, Philippe Mathieu-Daudé wrote: > On 12/11/19 5:05 PM, Damien Hedde wrote: >> Since we can now send packets of arbitrary length: >> simplify gdb_monitor_write() and send the whole payload >> in one packet. > > While we can send arbitrary length on the wire, we advertise > PacketSize=MAX_PACKET_LENGTH in handle_query_supported(). > > We can raise this limit however. Hi Philippe, This parameter is only about the packet size we can handle (packets we can receive). -- Damien
© 2016 - 2025 Red Hat, Inc.