[PATCH 06/11] gdbstub: pass GString to gdb_build_stop_packet

Alex Bennée posted 11 patches 6 days, 14 hours ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Laurent Vivier <laurent@vivier.eu>
[PATCH 06/11] gdbstub: pass GString to gdb_build_stop_packet
Posted by Alex Bennée 6 days, 14 hours ago
The other functions we are going to clean-up work variously with there
own dynamically allocated GStrings or with the common shared buffer.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub/internals.h | 5 ++++-
 gdbstub/gdbstub.c   | 4 ++--
 gdbstub/system.c    | 4 ++--
 gdbstub/user.c      | 8 ++++----
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index 3134a6e8eb2..9b25bf58b8e 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -239,9 +239,12 @@ int gdb_target_memory_rw_debug(CPUState *cs, hwaddr addr,
 
 /**
  * gdb_build_stop_packet() - craft the stop packet
+ * @buf: GString buffer for building the packet
  * @cs: CPUState
+ *
+ * Craft the Stop/Reply packet when we halt.
  */
 
-void gdb_build_stop_packet(CPUState *cs);
+void gdb_build_stop_packet(GString *buf, CPUState *cs);
 
 #endif /* GDBSTUB_INTERNALS_H */
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index c82fb5ad324..b45eb7c7b2b 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -1432,7 +1432,7 @@ static void handle_v_attach(GArray *params, void *user_ctx)
         gdbserver_state.c_cpu = cpu;
 
         if (gdbserver_state.allow_stop_reply) {
-            gdb_build_stop_packet(cpu);
+            gdb_build_stop_packet(gdbserver_state.str_buf, cpu);
             gdbserver_state.allow_stop_reply = false;
         }
     }
@@ -2036,7 +2036,7 @@ static void handle_gen_set(GArray *params, void *user_ctx)
 static void handle_target_halt(GArray *params, void *user_ctx)
 {
     if (gdbserver_state.allow_stop_reply) {
-        gdb_build_stop_packet(gdbserver_state.c_cpu);
+        gdb_build_stop_packet(gdbserver_state.str_buf, gdbserver_state.c_cpu);
         gdbserver_state.allow_stop_reply = false;
         gdb_put_strbuf();
     }
diff --git a/gdbstub/system.c b/gdbstub/system.c
index 6963c930b01..8ec8b7ea336 100644
--- a/gdbstub/system.c
+++ b/gdbstub/system.c
@@ -668,8 +668,8 @@ void gdb_breakpoint_remove_all(CPUState *cs)
  *   T05core:{id};
  */
 
-void gdb_build_stop_packet(CPUState *cs)
+void gdb_build_stop_packet(GString *buf, CPUState *cs)
 {
-    g_string_printf(gdbserver_state.str_buf,
+    g_string_printf(buf,
                     "T%02xcore:%02x;", GDB_SIGNAL_TRAP, gdb_get_cpu_index(cs));
 }
diff --git a/gdbstub/user.c b/gdbstub/user.c
index c7a3ef947ed..a16f37616b1 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -976,9 +976,9 @@ void gdb_handle_query_xfer_siginfo(GArray *params, void *user_ctx)
  *   T05thread:{id};
  */
 
-void gdb_build_stop_packet(CPUState *cs)
+void gdb_build_stop_packet(GString *buf, CPUState *cs)
 {
-    g_string_printf(gdbserver_state.str_buf, "T%02xthread:", GDB_SIGNAL_TRAP);
-    gdb_append_thread_id(cs, gdbserver_state.str_buf);
-    g_string_append_c(gdbserver_state.str_buf, ';');
+    g_string_printf(buf, "T%02xthread:", GDB_SIGNAL_TRAP);
+    gdb_append_thread_id(cs, buf);
+    g_string_append_c(buf, ';');
 }
-- 
2.47.3


Re: [PATCH 06/11] gdbstub: pass GString to gdb_build_stop_packet
Posted by Richard Henderson 5 days, 21 hours ago
On 2/3/26 21:51, Alex Bennée wrote:
> The other functions we are going to clean-up work variously with there
> own dynamically allocated GStrings or with the common shared buffer.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   gdbstub/internals.h | 5 ++++-
>   gdbstub/gdbstub.c   | 4 ++--
>   gdbstub/system.c    | 4 ++--
>   gdbstub/user.c      | 8 ++++----
>   4 files changed, 12 insertions(+), 9 deletions(-)

This function is brand new in patch 4.
Why not just merge these two patches?

Either way,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

> 
> diff --git a/gdbstub/internals.h b/gdbstub/internals.h
> index 3134a6e8eb2..9b25bf58b8e 100644
> --- a/gdbstub/internals.h
> +++ b/gdbstub/internals.h
> @@ -239,9 +239,12 @@ int gdb_target_memory_rw_debug(CPUState *cs, hwaddr addr,
>   
>   /**
>    * gdb_build_stop_packet() - craft the stop packet
> + * @buf: GString buffer for building the packet
>    * @cs: CPUState
> + *
> + * Craft the Stop/Reply packet when we halt.
>    */
>   
> -void gdb_build_stop_packet(CPUState *cs);
> +void gdb_build_stop_packet(GString *buf, CPUState *cs);
>   
>   #endif /* GDBSTUB_INTERNALS_H */
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index c82fb5ad324..b45eb7c7b2b 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -1432,7 +1432,7 @@ static void handle_v_attach(GArray *params, void *user_ctx)
>           gdbserver_state.c_cpu = cpu;
>   
>           if (gdbserver_state.allow_stop_reply) {
> -            gdb_build_stop_packet(cpu);
> +            gdb_build_stop_packet(gdbserver_state.str_buf, cpu);
>               gdbserver_state.allow_stop_reply = false;
>           }
>       }
> @@ -2036,7 +2036,7 @@ static void handle_gen_set(GArray *params, void *user_ctx)
>   static void handle_target_halt(GArray *params, void *user_ctx)
>   {
>       if (gdbserver_state.allow_stop_reply) {
> -        gdb_build_stop_packet(gdbserver_state.c_cpu);
> +        gdb_build_stop_packet(gdbserver_state.str_buf, gdbserver_state.c_cpu);
>           gdbserver_state.allow_stop_reply = false;
>           gdb_put_strbuf();
>       }
> diff --git a/gdbstub/system.c b/gdbstub/system.c
> index 6963c930b01..8ec8b7ea336 100644
> --- a/gdbstub/system.c
> +++ b/gdbstub/system.c
> @@ -668,8 +668,8 @@ void gdb_breakpoint_remove_all(CPUState *cs)
>    *   T05core:{id};
>    */
>   
> -void gdb_build_stop_packet(CPUState *cs)
> +void gdb_build_stop_packet(GString *buf, CPUState *cs)
>   {
> -    g_string_printf(gdbserver_state.str_buf,
> +    g_string_printf(buf,
>                       "T%02xcore:%02x;", GDB_SIGNAL_TRAP, gdb_get_cpu_index(cs));
>   }
> diff --git a/gdbstub/user.c b/gdbstub/user.c
> index c7a3ef947ed..a16f37616b1 100644
> --- a/gdbstub/user.c
> +++ b/gdbstub/user.c
> @@ -976,9 +976,9 @@ void gdb_handle_query_xfer_siginfo(GArray *params, void *user_ctx)
>    *   T05thread:{id};
>    */
>   
> -void gdb_build_stop_packet(CPUState *cs)
> +void gdb_build_stop_packet(GString *buf, CPUState *cs)
>   {
> -    g_string_printf(gdbserver_state.str_buf, "T%02xthread:", GDB_SIGNAL_TRAP);
> -    gdb_append_thread_id(cs, gdbserver_state.str_buf);
> -    g_string_append_c(gdbserver_state.str_buf, ';');
> +    g_string_printf(buf, "T%02xthread:", GDB_SIGNAL_TRAP);
> +    gdb_append_thread_id(cs, buf);
> +    g_string_append_c(buf, ';');
>   }


Re: [PATCH 06/11] gdbstub: pass GString to gdb_build_stop_packet
Posted by Alex Bennée 6 days, 10 hours ago
Alex Bennée <alex.bennee@linaro.org> writes:

> The other functions we are going to clean-up work variously with there

their...

> own dynamically allocated GStrings or with the common shared buffer.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  gdbstub/internals.h | 5 ++++-
>  gdbstub/gdbstub.c   | 4 ++--
>  gdbstub/system.c    | 4 ++--
>  gdbstub/user.c      | 8 ++++----
>  4 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/gdbstub/internals.h b/gdbstub/internals.h
> index 3134a6e8eb2..9b25bf58b8e 100644
> --- a/gdbstub/internals.h
> +++ b/gdbstub/internals.h
> @@ -239,9 +239,12 @@ int gdb_target_memory_rw_debug(CPUState *cs, hwaddr addr,
>  
>  /**
>   * gdb_build_stop_packet() - craft the stop packet
> + * @buf: GString buffer for building the packet
>   * @cs: CPUState
> + *
> + * Craft the Stop/Reply packet when we halt.
>   */
>  
> -void gdb_build_stop_packet(CPUState *cs);
> +void gdb_build_stop_packet(GString *buf, CPUState *cs);
>  
>  #endif /* GDBSTUB_INTERNALS_H */
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index c82fb5ad324..b45eb7c7b2b 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -1432,7 +1432,7 @@ static void handle_v_attach(GArray *params, void *user_ctx)
>          gdbserver_state.c_cpu = cpu;
>  
>          if (gdbserver_state.allow_stop_reply) {
> -            gdb_build_stop_packet(cpu);
> +            gdb_build_stop_packet(gdbserver_state.str_buf, cpu);
>              gdbserver_state.allow_stop_reply = false;
>          }
>      }
> @@ -2036,7 +2036,7 @@ static void handle_gen_set(GArray *params, void *user_ctx)
>  static void handle_target_halt(GArray *params, void *user_ctx)
>  {
>      if (gdbserver_state.allow_stop_reply) {
> -        gdb_build_stop_packet(gdbserver_state.c_cpu);
> +        gdb_build_stop_packet(gdbserver_state.str_buf, gdbserver_state.c_cpu);
>          gdbserver_state.allow_stop_reply = false;
>          gdb_put_strbuf();
>      }
> diff --git a/gdbstub/system.c b/gdbstub/system.c
> index 6963c930b01..8ec8b7ea336 100644
> --- a/gdbstub/system.c
> +++ b/gdbstub/system.c
> @@ -668,8 +668,8 @@ void gdb_breakpoint_remove_all(CPUState *cs)
>   *   T05core:{id};
>   */
>  
> -void gdb_build_stop_packet(CPUState *cs)
> +void gdb_build_stop_packet(GString *buf, CPUState *cs)
>  {
> -    g_string_printf(gdbserver_state.str_buf,
> +    g_string_printf(buf,
>                      "T%02xcore:%02x;", GDB_SIGNAL_TRAP, gdb_get_cpu_index(cs));
>  }
> diff --git a/gdbstub/user.c b/gdbstub/user.c
> index c7a3ef947ed..a16f37616b1 100644
> --- a/gdbstub/user.c
> +++ b/gdbstub/user.c
> @@ -976,9 +976,9 @@ void gdb_handle_query_xfer_siginfo(GArray *params, void *user_ctx)
>   *   T05thread:{id};
>   */
>  
> -void gdb_build_stop_packet(CPUState *cs)
> +void gdb_build_stop_packet(GString *buf, CPUState *cs)
>  {
> -    g_string_printf(gdbserver_state.str_buf, "T%02xthread:", GDB_SIGNAL_TRAP);
> -    gdb_append_thread_id(cs, gdbserver_state.str_buf);
> -    g_string_append_c(gdbserver_state.str_buf, ';');
> +    g_string_printf(buf, "T%02xthread:", GDB_SIGNAL_TRAP);
> +    gdb_append_thread_id(cs, buf);
> +    g_string_append_c(buf, ';');
>  }

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro