diff mbox series

[19/25] gdbstub: Replace gdb_regs with an array

Message ID 20231009164104.369749-20-alex.bennee@linaro.org
State Handled Elsewhere
Headers show
Series October maintainer omnibus pre-PR (tests, gdbstub, plugins) | expand

Commit Message

Alex Bennée Oct. 9, 2023, 4:40 p.m. UTC 
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>
---
 include/hw/core/cpu.h |  2 +-
 gdbstub/gdbstub.c     | 35 +++++++++++++++++++++--------------
 2 files changed, 22 insertions(+), 15 deletions(-)

Comments

Alistair Francis Oct. 11, 2023, 12:57 a.m. UTC | #1 
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 mbox series

Patch

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', "