Re: [PATCH v5 21/22] gdbstub: change GDBState.last_packet to GByteArray

2020-01-15 Thread Richard Henderson
On 1/14/20 5:09 AM, Alex Bennée wrote:
> From: Damien Hedde 
> 
> Remove the packet size upper limit by using a GByteArray
> instead of a statically allocated array for last_packet.
> Thus we can now send big packets.
> 
> Also remove the last_packet_len field and use last_packet->len
> instead.
> 
> Signed-off-by: Damien Hedde 
> Reviewed-by: Philippe Mathieu-Daudé 
> Message-Id: <20191211160514.58373-2-damien.he...@greensocs.com>
> Signed-off-by: Alex Bennée 
> ---
>  gdbstub.c | 39 +--
>  1 file changed, 21 insertions(+), 18 deletions(-)

Reviewed-by: Richard Henderson 


r~



[PATCH v5 21/22] gdbstub: change GDBState.last_packet to GByteArray

2020-01-14 Thread Alex Bennée
From: Damien Hedde 

Remove the packet size upper limit by using a GByteArray
instead of a statically allocated array for last_packet.
Thus we can now send big packets.

Also remove the last_packet_len field and use last_packet->len
instead.

Signed-off-by: Damien Hedde 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20191211160514.58373-2-damien.he...@greensocs.com>
Signed-off-by: Alex Bennée 
---
 gdbstub.c | 39 +--
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index dcfa4f1dd7..b9fb8c0a95 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -351,8 +351,7 @@ typedef struct GDBState {
 int line_buf_index;
 int line_sum; /* running checksum */
 int line_csum; /* checksum at the end of the packet */
-uint8_t last_packet[MAX_PACKET_LENGTH + 4];
-int last_packet_len;
+GByteArray *last_packet;
 int signal;
 #ifdef CONFIG_USER_ONLY
 int fd;
@@ -384,6 +383,7 @@ static void init_gdbserver_state(void)
 gdbserver_state.init = true;
 gdbserver_state.str_buf = g_string_new(NULL);
 gdbserver_state.mem_buf = g_byte_array_sized_new(MAX_PACKET_LENGTH);
+gdbserver_state.last_packet = g_byte_array_sized_new(MAX_PACKET_LENGTH + 
4);
 }
 
 #ifndef CONFIG_USER_ONLY
@@ -626,28 +626,29 @@ static void hexdump(const char *buf, int len,
 static int put_packet_binary(const char *buf, int len, bool dump)
 {
 int csum, i;
-uint8_t *p;
-uint8_t *ps = _state.last_packet[0];
+uint8_t footer[3];
 
 if (dump && trace_event_get_state_backends(TRACE_GDBSTUB_IO_BINARYREPLY)) {
 hexdump(buf, len, trace_gdbstub_io_binaryreply);
 }
 
 for(;;) {
-p = ps;
-*(p++) = '$';
-memcpy(p, buf, len);
-p += len;
+g_byte_array_set_size(gdbserver_state.last_packet, 0);
+g_byte_array_append(gdbserver_state.last_packet,
+(const uint8_t *) "$", 1);
+g_byte_array_append(gdbserver_state.last_packet,
+(const uint8_t *) buf, len);
 csum = 0;
 for(i = 0; i < len; i++) {
 csum += buf[i];
 }
-*(p++) = '#';
-*(p++) = tohex((csum >> 4) & 0xf);
-*(p++) = tohex((csum) & 0xf);
+footer[0] = '#';
+footer[1] = tohex((csum >> 4) & 0xf);
+footer[2] = tohex((csum) & 0xf);
+g_byte_array_append(gdbserver_state.last_packet, footer, 3);
 
-gdbserver_state.last_packet_len = p - ps;
-put_buffer(ps, gdbserver_state.last_packet_len);
+put_buffer(gdbserver_state.last_packet->data,
+   gdbserver_state.last_packet->len);
 
 #ifdef CONFIG_USER_ONLY
 i = get_char();
@@ -2812,20 +2813,22 @@ static void gdb_read_byte(uint8_t ch)
 uint8_t reply;
 
 #ifndef CONFIG_USER_ONLY
-if (gdbserver_state.last_packet_len) {
+if (gdbserver_state.last_packet->len) {
 /* Waiting for a response to the last packet.  If we see the start
of a new command then abandon the previous response.  */
 if (ch == '-') {
 trace_gdbstub_err_got_nack();
-put_buffer((uint8_t *)gdbserver_state.last_packet, 
gdbserver_state.last_packet_len);
+put_buffer(gdbserver_state.last_packet->data,
+   gdbserver_state.last_packet->len);
 } else if (ch == '+') {
 trace_gdbstub_io_got_ack();
 } else {
 trace_gdbstub_io_got_unexpected(ch);
 }
 
-if (ch == '+' || ch == '$')
-gdbserver_state.last_packet_len = 0;
+if (ch == '+' || ch == '$') {
+g_byte_array_set_size(gdbserver_state.last_packet, 0);
+}
 if (ch != '$')
 return;
 }
@@ -3209,7 +3212,7 @@ static int gdb_monitor_write(Chardev *chr, const uint8_t 
*buf, int len)
 const char *p = (const char *)buf;
 int max_sz;
 
-max_sz = (sizeof(gdbserver_state.last_packet) - 2) / 2;
+max_sz = (MAX_PACKET_LENGTH / 2) + 1;
 for (;;) {
 if (len <= max_sz) {
 gdb_monitor_output(p, len);
-- 
2.20.1