On Tue, Oct 10, 2023 at 2:47 AM Alex Bennée <alex.ben...@linaro.org> wrote:
>
> From: Akihiko Odaki <akihiko.od...@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.od...@daynix.com>
> Message-Id: <20230912224107.29669-13-akihiko.od...@daynix.com>
> [AJB: fixed a checkpatch violation]
> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
Reviewed-by: Alistair Francis <alistair.fran...@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
>
>
