| Message ID | 20231009164104.369749-20-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | October maintainer omnibus pre-PR (tests, gdbstub, plugins) | expand |
On Tue, Oct 10, 2023 at 2:47 AM Alex Bennée <alex.bennee@linaro.org> wrote: > > From: Akihiko Odaki <akihiko.odaki@daynix.com> > > An array is a more appropriate data structure than a list for gdb_regs > since it is initialized only with append operation and read-only after > initialization. > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > Message-Id: <20230912224107.29669-13-akihiko.odaki@daynix.com> > [AJB: fixed a checkpatch violation] > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > include/hw/core/cpu.h | 2 +- > gdbstub/gdbstub.c | 35 +++++++++++++++++++++-------------- > 2 files changed, 22 insertions(+), 15 deletions(-) > > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h > index 7b8347ed5a..3968369554 100644 > --- a/include/hw/core/cpu.h > +++ b/include/hw/core/cpu.h > @@ -502,7 +502,7 @@ struct CPUState { > > CPUJumpCache *tb_jmp_cache; > > - struct GDBRegisterState *gdb_regs; > + GArray *gdb_regs; > int gdb_num_regs; > int gdb_num_g_regs; > QTAILQ_ENTRY(CPUState) node; > diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c > index 62608a5389..b1532118d1 100644 > --- a/gdbstub/gdbstub.c > +++ b/gdbstub/gdbstub.c > @@ -51,7 +51,6 @@ typedef struct GDBRegisterState { > gdb_get_reg_cb get_reg; > gdb_set_reg_cb set_reg; > const char *xml; > - struct GDBRegisterState *next; > } GDBRegisterState; > > GDBState gdbserver_state; > @@ -386,7 +385,8 @@ static const char *get_feature_xml(const char *p, const char **newp, > xml, > g_markup_printf_escaped("<xi:include href=\"%s\"/>", > cc->gdb_core_xml_file)); > - for (r = cpu->gdb_regs; r; r = r->next) { > + for (guint i = 0; i < cpu->gdb_regs->len; i++) { > + r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); > g_ptr_array_add( > xml, > g_markup_printf_escaped("<xi:include href=\"%s\"/>", > @@ -430,7 +430,8 @@ static int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg) > return cc->gdb_read_register(cpu, buf, reg); > } > > - for (r = cpu->gdb_regs; r; r = r->next) { > + for (guint i = 0; i < cpu->gdb_regs->len; i++) { > + r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); > if (r->base_reg <= reg && reg < r->base_reg + r->num_regs) { > return r->get_reg(env, buf, reg - r->base_reg); > } > @@ -448,7 +449,8 @@ static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg) > return cc->gdb_write_register(cpu, mem_buf, reg); > } > > - for (r = cpu->gdb_regs; r; r = r->next) { > + for (guint i = 0; i < cpu->gdb_regs->len; i++) { > + r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); > if (r->base_reg <= reg && reg < r->base_reg + r->num_regs) { > return r->set_reg(env, mem_buf, reg - r->base_reg); > } > @@ -461,17 +463,23 @@ void gdb_register_coprocessor(CPUState *cpu, > int num_regs, const char *xml, int g_pos) > { > GDBRegisterState *s; > - GDBRegisterState **p; > - > - p = &cpu->gdb_regs; > - while (*p) { > - /* Check for duplicates. */ > - if (strcmp((*p)->xml, xml) == 0) > - return; > - p = &(*p)->next; > + guint i; > + > + if (cpu->gdb_regs) { > + for (i = 0; i < cpu->gdb_regs->len; i++) { > + /* Check for duplicates. */ > + s = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); > + if (strcmp(s->xml, xml) == 0) { > + return; > + } > + } > + } else { > + cpu->gdb_regs = g_array_new(false, false, sizeof(GDBRegisterState)); > + i = 0; > } > > - s = g_new0(GDBRegisterState, 1); > + g_array_set_size(cpu->gdb_regs, i + 1); > + s = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); > s->base_reg = cpu->gdb_num_regs; > s->num_regs = num_regs; > s->get_reg = get_reg; > @@ -480,7 +488,6 @@ void gdb_register_coprocessor(CPUState *cpu, > > /* Add to end of list. */ > cpu->gdb_num_regs += num_regs; > - *p = s; > if (g_pos) { > if (g_pos != s->base_reg) { > error_report("Error: Bad gdb register numbering for '%s', " > -- > 2.39.2 > >
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index 7b8347ed5a..3968369554 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -502,7 +502,7 @@ struct CPUState { CPUJumpCache *tb_jmp_cache; - struct GDBRegisterState *gdb_regs; + GArray *gdb_regs; int gdb_num_regs; int gdb_num_g_regs; QTAILQ_ENTRY(CPUState) node; diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index 62608a5389..b1532118d1 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -51,7 +51,6 @@ typedef struct GDBRegisterState { gdb_get_reg_cb get_reg; gdb_set_reg_cb set_reg; const char *xml; - struct GDBRegisterState *next; } GDBRegisterState; GDBState gdbserver_state; @@ -386,7 +385,8 @@ static const char *get_feature_xml(const char *p, const char **newp, xml, g_markup_printf_escaped("<xi:include href=\"%s\"/>", cc->gdb_core_xml_file)); - for (r = cpu->gdb_regs; r; r = r->next) { + for (guint i = 0; i < cpu->gdb_regs->len; i++) { + r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); g_ptr_array_add( xml, g_markup_printf_escaped("<xi:include href=\"%s\"/>", @@ -430,7 +430,8 @@ static int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg) return cc->gdb_read_register(cpu, buf, reg); } - for (r = cpu->gdb_regs; r; r = r->next) { + for (guint i = 0; i < cpu->gdb_regs->len; i++) { + r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); if (r->base_reg <= reg && reg < r->base_reg + r->num_regs) { return r->get_reg(env, buf, reg - r->base_reg); } @@ -448,7 +449,8 @@ static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg) return cc->gdb_write_register(cpu, mem_buf, reg); } - for (r = cpu->gdb_regs; r; r = r->next) { + for (guint i = 0; i < cpu->gdb_regs->len; i++) { + r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); if (r->base_reg <= reg && reg < r->base_reg + r->num_regs) { return r->set_reg(env, mem_buf, reg - r->base_reg); } @@ -461,17 +463,23 @@ void gdb_register_coprocessor(CPUState *cpu, int num_regs, const char *xml, int g_pos) { GDBRegisterState *s; - GDBRegisterState **p; - - p = &cpu->gdb_regs; - while (*p) { - /* Check for duplicates. */ - if (strcmp((*p)->xml, xml) == 0) - return; - p = &(*p)->next; + guint i; + + if (cpu->gdb_regs) { + for (i = 0; i < cpu->gdb_regs->len; i++) { + /* Check for duplicates. */ + s = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); + if (strcmp(s->xml, xml) == 0) { + return; + } + } + } else { + cpu->gdb_regs = g_array_new(false, false, sizeof(GDBRegisterState)); + i = 0; } - s = g_new0(GDBRegisterState, 1); + g_array_set_size(cpu->gdb_regs, i + 1); + s = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); s->base_reg = cpu->gdb_num_regs; s->num_regs = num_regs; s->get_reg = get_reg; @@ -480,7 +488,6 @@ void gdb_register_coprocessor(CPUState *cpu, /* Add to end of list. */ cpu->gdb_num_regs += num_regs; - *p = s; if (g_pos) { if (g_pos != s->base_reg) { error_report("Error: Bad gdb register numbering for '%s', "