On 8/24/23 09:39, Alex Bennée wrote:
Try to bring up the code to more modern standards by:

   - use dynamic GString built xml over a fixed buffer
   - use autofree to save on explicit g_free() calls
   - don't hand hack strstr to find the delimiter

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

---
v2
   - avoid needless g_strndup for copy of annex
---
  gdbstub/internals.h |  2 +-
  gdbstub/gdbstub.c   | 63 +++++++++++++++++++++------------------------
  2 files changed, 31 insertions(+), 34 deletions(-)

diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index f2b46cce41..4876ebd74f 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -33,7 +33,7 @@ typedef struct GDBProcess {
      uint32_t pid;
      bool attached;
- char target_xml[1024];
+    char *target_xml;
  } GDBProcess;
enum RSState {
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 8e9bc17e07..31a2451f27 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -354,54 +354,51 @@ static CPUState *gdb_get_cpu(uint32_t pid, uint32_t tid)
  static const char *get_feature_xml(const char *p, const char **newp,
                                     GDBProcess *process)
  {
-    size_t len;
      int i;
      const char *name;
      CPUState *cpu = gdb_get_first_cpu_in_process(process);
      CPUClass *cc = CPU_GET_CLASS(cpu);
- len = 0;
-    while (p[len] && p[len] != ':')
-        len++;
-    *newp = p + len;
+    /* ‘qXfer:features:read:annex:offset,length' */

This is misleading, because "...:read:" has already been consumed.

+    char *term = g_strstr_len(p, -1, ":");

This is strchr(p, ':').

+    g_autofree char *annex = g_strndup(p, len);
+    /* leave newp at offset,length for the rest of the params */
+    *newp = term + 1;
- name = NULL;
-    if (strncmp(p, "target.xml", len) == 0) {
-        char *buf = process->target_xml;
-        const size_t buf_sz = sizeof(process->target_xml);
- /* Generate the XML description for this CPU. */
-        if (!buf[0]) {
+    name = NULL;
+    if (g_strcmp0(annex, "target.xml") == 0) {

Why the change to g_strcmp0?  There's no null pointer to be handled.
If you keep the strncmp, you don't have to allocate memory early.

      if (cc->gdb_get_dynamic_xml) {
-        char *xmlname = g_strndup(p, len);
-        const char *xml = cc->gdb_get_dynamic_xml(cpu, xmlname);
-
-        g_free(xmlname);
+        const char *xml = cc->gdb_get_dynamic_xml(cpu, annex);

You can leave the g_strndup (and autofree) to here.



r~

Reply via email to