diff mbox series

[v3,18/24] gdbstub: don't use target_ulong while handling registers

Message ID 20230221225227.3735319-19-alex.bennee@linaro.org
State New
Headers show
Series gdbstub: re-organise to for better compilation behaviour | expand

Commit Message

Alex Bennée Feb. 21, 2023, 10:52 p.m. UTC 
This is a hangover from the original code. addr is misleading as it is
only really a register id. While len will never exceed
MAX_PACKET_LENGTH I've used size_t as that is what strlen returns.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v3
  - fix commit message
  - use unsigned for regid
---
 gdbstub/gdbstub.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

Comments

Richard Henderson Feb. 22, 2023, 12:51 a.m. UTC | #1 
On 2/21/23 12:52, Alex Bennée wrote:
> This is a hangover from the original code. addr is misleading as it is
> only really a register id. While len will never exceed
> MAX_PACKET_LENGTH I've used size_t as that is what strlen returns.
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> v3
>    - fix commit message
>    - use unsigned for regid

Don't use unsigned, which you did here:

>   static void handle_write_all_regs(GArray *params, void *user_ctx)
>   {
> -    target_ulong addr, len;
> +    unsigned int reg_id;

but not here

>   static void handle_read_all_regs(GArray *params, void *user_ctx)
>   {
> -    target_ulong addr, len;
> +    int reg_id;

because the comparison,

> +    for (reg_id = 0; reg_id < gdbserver_state.g_cpu->gdb_num_g_regs; reg_id++) {

is against signed:

include/hw/core/cpu.h:377:    int gdb_num_g_regs;


r~
diff mbox series

Patch

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 4814e8fbf3..7ae3ff52b3 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -1192,7 +1192,8 @@  static void handle_read_mem(GArray *params, void *user_ctx)
 
 static void handle_write_all_regs(GArray *params, void *user_ctx)
 {
-    target_ulong addr, len;
+    unsigned int reg_id;
+    size_t len;
     uint8_t *registers;
     int reg_size;
 
@@ -1204,9 +1205,10 @@  static void handle_write_all_regs(GArray *params, void *user_ctx)
     len = strlen(get_param(params, 0)->data) / 2;
     gdb_hextomem(gdbserver_state.mem_buf, get_param(params, 0)->data, len);
     registers = gdbserver_state.mem_buf->data;
-    for (addr = 0; addr < gdbserver_state.g_cpu->gdb_num_g_regs && len > 0;
-         addr++) {
-        reg_size = gdb_write_register(gdbserver_state.g_cpu, registers, addr);
+    for (reg_id = 0;
+         reg_id < gdbserver_state.g_cpu->gdb_num_g_regs && len > 0;
+         reg_id++) {
+        reg_size = gdb_write_register(gdbserver_state.g_cpu, registers, reg_id);
         len -= reg_size;
         registers += reg_size;
     }
@@ -1215,15 +1217,16 @@  static void handle_write_all_regs(GArray *params, void *user_ctx)
 
 static void handle_read_all_regs(GArray *params, void *user_ctx)
 {
-    target_ulong addr, len;
+    int reg_id;
+    size_t len;
 
     cpu_synchronize_state(gdbserver_state.g_cpu);
     g_byte_array_set_size(gdbserver_state.mem_buf, 0);
     len = 0;
-    for (addr = 0; addr < gdbserver_state.g_cpu->gdb_num_g_regs; addr++) {
+    for (reg_id = 0; reg_id < gdbserver_state.g_cpu->gdb_num_g_regs; reg_id++) {
         len += gdb_read_register(gdbserver_state.g_cpu,
                                  gdbserver_state.mem_buf,
-                                 addr);
+                                 reg_id);
     }
     g_assert(len == gdbserver_state.mem_buf->len);