diff mbox series

[RFC,03/11] gdbstub: move str_buf to GDBState and use GString

Message ID 20191115173000.21891-4-alex.bennee@linaro.org
State New
Headers show
Series gdbstub re-factor and SVE support | expand

Commit Message

Alex Bennée Nov. 15, 2019, 5:29 p.m. UTC 
Rather than having a static buffer replace str_buf with a GString
which we know can grow on demand. Convert the internal functions to
take a GString instead of a char * and length.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
 gdbstub.c | 194 ++++++++++++++++++++++++------------------------------
 1 file changed, 85 insertions(+), 109 deletions(-)

-- 
2.20.1

Comments

Richard Henderson Nov. 18, 2019, 8:06 a.m. UTC | #1 
On 11/15/19 6:29 PM, Alex Bennée wrote:
> Rather than having a static buffer replace str_buf with a GString

> which we know can grow on demand. Convert the internal functions to

> take a GString instead of a char * and length.

> 

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

> ---

>  gdbstub.c | 194 ++++++++++++++++++++++++------------------------------

>  1 file changed, 85 insertions(+), 109 deletions(-)


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>



r~
diff mbox series

Patch

diff --git a/gdbstub.c b/gdbstub.c
index 2e6ff5f583c..528404c1953 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -365,6 +365,7 @@  typedef struct GDBState {
     int process_num;
     char syscall_buf[256];
     gdb_syscall_complete_cb current_syscall_cb;
+    GString *str_buf;
 } GDBState;
 
 /* By default use no IRQs and no timers while single stepping so as to
@@ -378,6 +379,7 @@  static GDBState *gdb_allocate_state(void)
 {
     g_assert(!gdbserver_state);
     gdbserver_state = g_new0(GDBState, 1);
+    gdbserver_state->str_buf = g_string_new(NULL);
     return gdbserver_state;
 }
 
@@ -553,17 +555,15 @@  static inline int tohex(int v)
 }
 
 /* writes 2*len+1 bytes in buf */
-static void memtohex(char *buf, const uint8_t *mem, int len)
+static void memtohex(GString *buf, const uint8_t *mem, int len)
 {
     int i, c;
-    char *q;
-    q = buf;
     for(i = 0; i < len; i++) {
         c = mem[i];
-        *q++ = tohex(c >> 4);
-        *q++ = tohex(c & 0xf);
+        g_string_append_c(buf, tohex(c >> 4));
+        g_string_append_c(buf, tohex(c & 0xf));
     }
-    *q = '\0';
+    g_string_append_c(buf, '\0');
 }
 
 static void hextomem(uint8_t *mem, const char *buf, int len)
@@ -657,24 +657,22 @@  static int put_packet(GDBState *s, const char *buf)
 }
 
 /* Encode data using the encoding for 'x' packets.  */
-static int memtox(char *buf, const char *mem, int len)
+static void memtox(GString *buf, const char *mem, int len)
 {
-    char *p = buf;
     char c;
 
     while (len--) {
         c = *(mem++);
         switch (c) {
         case '#': case '$': case '*': case '}':
-            *(p++) = '}';
-            *(p++) = c ^ 0x20;
+            g_string_append_c(buf, '}');
+            g_string_append_c(buf, c ^ 0x20);
             break;
         default:
-            *(p++) = c;
+            g_string_append_c(buf, c);
             break;
         }
     }
-    return p - buf;
 }
 
 static uint32_t gdb_get_cpu_pid(const GDBState *s, CPUState *cpu)
@@ -1098,17 +1096,14 @@  static void gdb_set_cpu_pc(GDBState *s, target_ulong pc)
     cpu_set_pc(cpu, pc);
 }
 
-static char *gdb_fmt_thread_id(const GDBState *s, CPUState *cpu,
-                           char *buf, size_t buf_size)
+static void gdb_append_thread_id(const GDBState *s, CPUState *cpu, GString *buf)
 {
     if (s->multiprocess) {
-        snprintf(buf, buf_size, "p%02x.%02x",
-                 gdb_get_cpu_pid(s, cpu), cpu_gdb_index(cpu));
+        g_string_append_printf(buf, "p%02x.%02x",
+                               gdb_get_cpu_pid(s, cpu), cpu_gdb_index(cpu));
     } else {
-        snprintf(buf, buf_size, "%02x", cpu_gdb_index(cpu));
+        g_string_append_printf(buf, "%02x", cpu_gdb_index(cpu));
     }
-
-    return buf;
 }
 
 typedef enum GDBThreadIdKind {
@@ -1402,7 +1397,6 @@  typedef struct GdbCmdContext {
     GdbCmdVariant *params;
     int num_params;
     uint8_t mem_buf[MAX_PACKET_LENGTH];
-    char str_buf[MAX_PACKET_LENGTH + 1];
 } GdbCmdContext;
 
 typedef void (*GdbCmdHandler)(GdbCmdContext *gdb_ctx, void *user_ctx);
@@ -1494,6 +1488,8 @@  static void run_cmd_parser(GDBState *s, const char *data,
         return;
     }
 
+    g_string_set_size(s->str_buf, 0);
+
     /* In case there was an error during the command parsing we must
     * send a NULL packet to indicate the command is not supported */
     if (process_string_cmd(s, NULL, data, cmd, 1)) {
@@ -1742,8 +1738,8 @@  static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
         return;
     }
 
-    memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, reg_size);
-    put_packet(s, gdb_ctx->str_buf);
+    memtohex(s->str_buf, gdb_ctx->mem_buf, reg_size);
+    put_packet(s, s->str_buf->str);
 }
 
 static void handle_write_mem(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -1795,8 +1791,8 @@  static void handle_read_mem(GdbCmdContext *gdb_ctx, void *user_ctx)
         return;
     }
 
-    memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, gdb_ctx->params[1].val_ull);
-    put_packet(s, gdb_ctx->str_buf);
+    memtohex(s->str_buf, gdb_ctx->mem_buf, gdb_ctx->params[1].val_ull);
+    put_packet(s, s->str_buf->str);
 }
 
 static void handle_write_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -1835,8 +1831,8 @@  static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
                                  addr);
     }
 
-    memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, len);
-    put_packet(s, gdb_ctx->str_buf);
+    memtohex(s->str_buf, gdb_ctx->mem_buf, len);
+    put_packet(s, s->str_buf->str);
 }
 
 static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -1904,9 +1900,8 @@  static void handle_v_attach(GdbCmdContext *gdb_ctx, void *user_ctx)
     GDBState *s = gdbserver_state;
     GDBProcess *process;
     CPUState *cpu;
-    char thread_id[16];
 
-    pstrcpy(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "E22");
+    g_string_assign(s->str_buf, "E22");
     if (!gdb_ctx->num_params) {
         goto cleanup;
     }
@@ -1925,11 +1920,11 @@  static void handle_v_attach(GdbCmdContext *gdb_ctx, void *user_ctx)
     s->g_cpu = cpu;
     s->c_cpu = cpu;
 
-    gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id));
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "T%02xthread:%s;",
-             GDB_SIGNAL_TRAP, thread_id);
+    g_string_printf(s->str_buf, "T%02xthread:", GDB_SIGNAL_TRAP);
+    gdb_append_thread_id(s, cpu, s->str_buf);
+    g_string_append_c(s->str_buf, ';');
 cleanup:
-    put_packet(s, gdb_ctx->str_buf);
+    put_packet(s, s->str_buf->str);
 }
 
 static void handle_v_kill(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -1986,11 +1981,9 @@  static void handle_v_commands(GdbCmdContext *gdb_ctx, void *user_ctx)
 static void handle_query_qemu_sstepbits(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     GDBState *s = gdbserver_state;
-
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf),
-             "ENABLE=%x,NOIRQ=%x,NOTIMER=%x", SSTEP_ENABLE,
-             SSTEP_NOIRQ, SSTEP_NOTIMER);
-    put_packet(s, gdb_ctx->str_buf);
+    g_string_printf(s->str_buf, "ENABLE=%x,NOIRQ=%x,NOTIMER=%x",
+                    SSTEP_ENABLE, SSTEP_NOIRQ, SSTEP_NOTIMER);
+    put_packet(s, s->str_buf->str);
 }
 
 static void handle_set_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -2008,9 +2001,8 @@  static void handle_set_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx)
 static void handle_query_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     GDBState *s = gdbserver_state;
-
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "0x%x", sstep_flags);
-    put_packet(s, gdb_ctx->str_buf);
+    g_string_printf(s->str_buf, "0x%x", sstep_flags);
+    put_packet(s, s->str_buf->str);
 }
 
 static void handle_query_curr_tid(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -2018,7 +2010,6 @@  static void handle_query_curr_tid(GdbCmdContext *gdb_ctx, void *user_ctx)
     GDBState *s = gdbserver_state;
     CPUState *cpu;
     GDBProcess *process;
-    char thread_id[16];
 
     /*
      * "Current thread" remains vague in the spec, so always return
@@ -2027,27 +2018,24 @@  static void handle_query_curr_tid(GdbCmdContext *gdb_ctx, void *user_ctx)
      */
     process = gdb_get_cpu_process(s, s->g_cpu);
     cpu = get_first_cpu_in_process(s, process);
-    gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id));
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "QC%s", thread_id);
-    put_packet(s, gdb_ctx->str_buf);
+    g_string_assign(s->str_buf, "QC");
+    gdb_append_thread_id(s, cpu, s->str_buf);
+    put_packet(s, s->str_buf->str);
 }
 
 static void handle_query_threads(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     GDBState *s = gdbserver_state;
-    char thread_id[16];
 
     if (!s->query_cpu) {
         put_packet(s, "l");
         return;
     }
 
-    gdb_fmt_thread_id(s, s->query_cpu, thread_id,
-                      sizeof(thread_id));
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "m%s", thread_id);
-    put_packet(s, gdb_ctx->str_buf);
-    s->query_cpu =
-        gdb_next_attached_cpu(s, s->query_cpu);
+    g_string_assign(s->str_buf, "m");
+    gdb_append_thread_id(s, s->query_cpu, s->str_buf);
+    put_packet(s, s->str_buf->str);
+    s->query_cpu = gdb_next_attached_cpu(s, s->query_cpu);
 }
 
 static void handle_query_first_threads(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -2060,8 +2048,8 @@  static void handle_query_first_threads(GdbCmdContext *gdb_ctx, void *user_ctx)
 static void handle_query_thread_extra(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     GDBState *s = gdbserver_state;
+    g_autoptr(GString) rs = g_string_new(NULL);
     CPUState *cpu;
-    int len;
 
     if (!gdb_ctx->num_params ||
         gdb_ctx->params[0].thread_id.kind == GDB_READ_THREAD_ERR) {
@@ -2081,20 +2069,17 @@  static void handle_query_thread_extra(GdbCmdContext *gdb_ctx, void *user_ctx)
         /* Print the CPU model and name in multiprocess mode */
         ObjectClass *oc = object_get_class(OBJECT(cpu));
         const char *cpu_model = object_class_get_name(oc);
-        char *cpu_name = object_get_canonical_path_component(OBJECT(cpu));
-        len = snprintf((char *)gdb_ctx->mem_buf, sizeof(gdb_ctx->str_buf) / 2,
-                       "%s %s [%s]", cpu_model, cpu_name,
-                       cpu->halted ? "halted " : "running");
-        g_free(cpu_name);
+        g_autofree char *cpu_name;
+        cpu_name  = object_get_canonical_path_component(OBJECT(cpu));
+        g_string_printf(rs, "%s %s [%s]", cpu_model, cpu_name,
+                        cpu->halted ? "halted " : "running");
     } else {
-        /* memtohex() doubles the required space */
-        len = snprintf((char *)gdb_ctx->mem_buf, sizeof(gdb_ctx->str_buf) / 2,
-                        "CPU#%d [%s]", cpu->cpu_index,
+        g_string_printf(rs, "CPU#%d [%s]", cpu->cpu_index,
                         cpu->halted ? "halted " : "running");
     }
-    trace_gdbstub_op_extra_info((char *)gdb_ctx->mem_buf);
-    memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, len);
-    put_packet(s, gdb_ctx->str_buf);
+    trace_gdbstub_op_extra_info(rs->str);
+    memtohex(s->str_buf, (uint8_t *)rs->str, rs->len);
+    put_packet(s, s->str_buf->str);
 }
 
 #ifdef CONFIG_USER_ONLY
@@ -2104,13 +2089,14 @@  static void handle_query_offsets(GdbCmdContext *gdb_ctx, void *user_ctx)
     TaskState *ts;
 
     ts = s->c_cpu->opaque;
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf),
-             "Text=" TARGET_ABI_FMT_lx ";Data=" TARGET_ABI_FMT_lx
-             ";Bss=" TARGET_ABI_FMT_lx,
-             ts->info->code_offset,
-             ts->info->data_offset,
-             ts->info->data_offset);
-    put_packet(s, gdb_ctx->str_buf);
+    g_string_printf(s->str_buf,
+                    "Text=" TARGET_ABI_FMT_lx
+                    ";Data=" TARGET_ABI_FMT_lx
+                    ";Bss=" TARGET_ABI_FMT_lx,
+                    ts->info->code_offset,
+                    ts->info->data_offset,
+                    ts->info->data_offset);
+    put_packet(s, s->str_buf->str);
 }
 #else
 static void handle_query_rcmd(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -2143,12 +2129,10 @@  static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
     GDBState *s = gdbserver_state;
     CPUClass *cc;
 
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "PacketSize=%x",
-             MAX_PACKET_LENGTH);
+    g_string_printf(s->str_buf, "PacketSize=%x", MAX_PACKET_LENGTH);
     cc = CPU_GET_CLASS(first_cpu);
     if (cc->gdb_core_xml_file) {
-        pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf),
-                ";qXfer:features:read+");
+        g_string_append(s->str_buf, ";qXfer:features:read+");
     }
 
     if (gdb_ctx->num_params &&
@@ -2156,8 +2140,8 @@  static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
         s->multiprocess = true;
     }
 
-    pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";multiprocess+");
-    put_packet(s, gdb_ctx->str_buf);
+    g_string_append(s->str_buf, ";multiprocess+");
+    put_packet(s, s->str_buf->str);
 }
 
 static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -2202,14 +2186,14 @@  static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx)
     }
 
     if (len < total_len - addr) {
-        gdb_ctx->str_buf[0] = 'm';
-        len = memtox(gdb_ctx->str_buf + 1, xml + addr, len);
+        g_string_assign(s->str_buf, "m");
+        memtox(s->str_buf, xml + addr, len);
     } else {
-        gdb_ctx->str_buf[0] = 'l';
-        len = memtox(gdb_ctx->str_buf + 1, xml + addr, total_len - addr);
+        g_string_assign(s->str_buf, "l");
+        memtox(s->str_buf, xml + addr, total_len - addr);
     }
 
-    put_packet_binary(s, gdb_ctx->str_buf, len + 1, true);
+    put_packet_binary(s, s->str_buf->str, s->str_buf->len, true);
 }
 
 static void handle_query_attached(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -2221,11 +2205,11 @@  static void handle_query_attached(GdbCmdContext *gdb_ctx, void *user_ctx)
 static void handle_query_qemu_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     GDBState *s = gdbserver_state;
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "sstepbits;sstep");
+    g_string_printf(s->str_buf, "sstepbits;sstep");
 #ifndef CONFIG_USER_ONLY
-    pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";PhyMemMode");
+    g_string_append(s->str_buf, ";PhyMemMode");
 #endif
-    put_packet(s, gdb_ctx->str_buf);
+    put_packet(s, s->str_buf->str);
 }
 
 #ifndef CONFIG_USER_ONLY
@@ -2233,8 +2217,8 @@  static void handle_query_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx,
                                            void *user_ctx)
 {
     GDBState *s = gdbserver_state;
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "%d", phy_memory_mode);
-    put_packet(s, gdb_ctx->str_buf);
+    g_string_printf(s->str_buf, "%d", phy_memory_mode);
+    put_packet(s, s->str_buf->str);
 }
 
 static void handle_set_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx, void *user_ctx)
@@ -2405,13 +2389,10 @@  static void handle_gen_set(GdbCmdContext *gdb_ctx, void *user_ctx)
 static void handle_target_halt(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     GDBState *s = gdbserver_state;
-    char thread_id[16];
-
-    gdb_fmt_thread_id(s, s->c_cpu, thread_id,
-                      sizeof(thread_id));
-    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "T%02xthread:%s;",
-             GDB_SIGNAL_TRAP, thread_id);
-    put_packet(s, gdb_ctx->str_buf);
+    g_string_printf(s->str_buf, "T%02xthread:", GDB_SIGNAL_TRAP);
+    gdb_append_thread_id(s, s->c_cpu, s->str_buf);
+    g_string_append_c(s->str_buf, ';');
+    put_packet(s, s->str_buf->str);
     /*
      * Remove all the breakpoints when this query is issued,
      * because gdb is doing an initial connect and the state
@@ -2675,8 +2656,8 @@  static void gdb_vm_state_change(void *opaque, int running, RunState state)
 {
     GDBState *s = gdbserver_state;
     CPUState *cpu = s->c_cpu;
-    char buf[256];
-    char thread_id[16];
+    g_autoptr(GString) buf = g_string_new(NULL);
+    g_autoptr(GString) tid = g_string_new(NULL);
     const char *type;
     int ret;
 
@@ -2694,7 +2675,7 @@  static void gdb_vm_state_change(void *opaque, int running, RunState state)
         return;
     }
 
-    gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id));
+    gdb_append_thread_id(s, cpu, tid);
 
     switch (state) {
     case RUN_STATE_DEBUG:
@@ -2712,10 +2693,9 @@  static void gdb_vm_state_change(void *opaque, int running, RunState state)
             }
             trace_gdbstub_hit_watchpoint(type, cpu_gdb_index(cpu),
                     (target_ulong)cpu->watchpoint_hit->vaddr);
-            snprintf(buf, sizeof(buf),
-                     "T%02xthread:%s;%swatch:" TARGET_FMT_lx ";",
-                     GDB_SIGNAL_TRAP, thread_id, type,
-                     (target_ulong)cpu->watchpoint_hit->vaddr);
+            g_string_printf(buf, "T%02xthread:%s;%swatch:" TARGET_FMT_lx ";",
+                            GDB_SIGNAL_TRAP, tid->str, type,
+                            (target_ulong)cpu->watchpoint_hit->vaddr);
             cpu->watchpoint_hit = NULL;
             goto send_packet;
         } else {
@@ -2756,10 +2736,10 @@  static void gdb_vm_state_change(void *opaque, int running, RunState state)
         break;
     }
     gdb_set_stop_cpu(cpu);
-    snprintf(buf, sizeof(buf), "T%02xthread:%s;", ret, thread_id);
+    g_string_printf(buf, "T%02xthread:%s;", ret, tid->str);
 
 send_packet:
-    put_packet(s, buf);
+    put_packet(s, buf->str);
 
     /* disable single step if it was enabled */
     cpu_single_step(cpu, 0);
@@ -3248,13 +3228,9 @@  static void gdb_chr_event(void *opaque, int event)
 
 static void gdb_monitor_output(GDBState *s, const char *msg, int len)
 {
-    char buf[MAX_PACKET_LENGTH];
-
-    buf[0] = 'O';
-    if (len > (MAX_PACKET_LENGTH/2) - 1)
-        len = (MAX_PACKET_LENGTH/2) - 1;
-    memtohex(buf + 1, (uint8_t *)msg, len);
-    put_packet(s, buf);
+    g_autoptr(GString) buf = g_string_new("O");
+    memtohex(buf, (uint8_t *)msg, len);
+    put_packet(s, buf->str);
 }
 
 static int gdb_monitor_write(Chardev *chr, const uint8_t *buf, int len)