gdbstub: refactor get_feature_xml
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
- fix up style of xml_builtin and invert loop
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20230829161528.2707696-11-alex.bennee@linaro.org>
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 8e9bc17..729e541 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -354,64 +354,67 @@
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);
+ size_t len;
- len = 0;
- while (p[len] && p[len] != ':')
- len++;
- *newp = p + len;
+ /*
+ * qXfer:features:read:ANNEX:OFFSET,LENGTH'
+ * ^p ^newp
+ */
+ char *term = strchr(p, ':');
+ *newp = term + 1;
+ len = term - p;
- name = NULL;
+ /* Is it the main target xml? */
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]) {
+ if (!process->target_xml) {
GDBRegisterState *r;
+ GString *xml = g_string_new("<?xml version=\"1.0\"?>");
- pstrcat(buf, buf_sz,
- "<?xml version=\"1.0\"?>"
- "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
- "<target>");
+ g_string_append(xml,
+ "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
+ "<target>");
+
if (cc->gdb_arch_name) {
- gchar *arch = cc->gdb_arch_name(cpu);
- pstrcat(buf, buf_sz, "<architecture>");
- pstrcat(buf, buf_sz, arch);
- pstrcat(buf, buf_sz, "</architecture>");
- g_free(arch);
+ g_autofree gchar *arch = cc->gdb_arch_name(cpu);
+ g_string_append_printf(xml,
+ "<architecture>%s</architecture>",
+ arch);
}
- pstrcat(buf, buf_sz, "<xi:include href=\"");
- pstrcat(buf, buf_sz, cc->gdb_core_xml_file);
- pstrcat(buf, buf_sz, "\"/>");
+ g_string_append(xml, "<xi:include href=\"");
+ g_string_append(xml, cc->gdb_core_xml_file);
+ g_string_append(xml, "\"/>");
for (r = cpu->gdb_regs; r; r = r->next) {
- pstrcat(buf, buf_sz, "<xi:include href=\"");
- pstrcat(buf, buf_sz, r->xml);
- pstrcat(buf, buf_sz, "\"/>");
+ g_string_append(xml, "<xi:include href=\"");
+ g_string_append(xml, r->xml);
+ g_string_append(xml, "\"/>");
}
- pstrcat(buf, buf_sz, "</target>");
- }
- return buf;
- }
- if (cc->gdb_get_dynamic_xml) {
- char *xmlname = g_strndup(p, len);
- const char *xml = cc->gdb_get_dynamic_xml(cpu, xmlname);
+ g_string_append(xml, "</target>");
- g_free(xmlname);
+ process->target_xml = g_string_free(xml, false);
+ return process->target_xml;
+ }
+ }
+ /* Is it dynamically generated by the target? */
+ if (cc->gdb_get_dynamic_xml) {
+ g_autofree char *xmlname = g_strndup(p, len);
+ const char *xml = cc->gdb_get_dynamic_xml(cpu, xmlname);
if (xml) {
return xml;
}
}
- for (i = 0; ; i++) {
- name = xml_builtin[i][0];
- if (!name || (strncmp(name, p, len) == 0 && strlen(name) == len))
- break;
+ /* Is it one of the encoded gdb-xml/ files? */
+ for (int i = 0; xml_builtin[i][0]; i++) {
+ const char *name = xml_builtin[i][0];
+ if ((strncmp(name, p, len) == 0) &&
+ strlen(name) == len) {
+ return xml_builtin[i][1];
+ }
}
- return name ? xml_builtin[i][1] : NULL;
+
+ /* failed */
+ return NULL;
}
static int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg)
@@ -2245,6 +2248,6 @@
process = &s->processes[s->process_num - 1];
process->pid = pid;
process->attached = false;
- process->target_xml[0] = '\0';
+ process->target_xml = NULL;
}
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index f2b46cc..4876ebd 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -33,7 +33,7 @@
uint32_t pid;
bool attached;
- char target_xml[1024];
+ char *target_xml;
} GDBProcess;
enum RSState {
