[02/26] gdbserver: convert new_register_cache into a regcache constructor

Message ID f1dad7ea7da2e24e4629dad5e0a641e70ba9e9dd.1677582744.git.tankut.baris.aktemur@intel.com
State New
Headers
Series gdbserver: refactor regcache and allow gradually populating |

Commit Message

Tankut Baris Aktemur Feb. 28, 2023, 11:28 a.m. UTC
  Convert the `new_register_cache` function into a constructor of the
regcache struct.  Also define a default ctor because it is used in
other places, in particular, tracepoint.cc.
---
 gdbserver/regcache.cc | 11 +++--------
 gdbserver/regcache.h  |  8 ++++----
 gdbserver/server.cc   |  2 +-
 3 files changed, 8 insertions(+), 13 deletions(-)
  

Comments

Simon Marchi Dec. 21, 2023, 8:19 p.m. UTC | #1
On 2/28/23 06:28, Tankut Baris Aktemur via Gdb-patches wrote:
> Convert the `new_register_cache` function into a constructor of the
> regcache struct.  Also define a default ctor because it is used in
> other places, in particular, tracepoint.cc.
> ---
>  gdbserver/regcache.cc | 11 +++--------
>  gdbserver/regcache.h  |  8 ++++----
>  gdbserver/server.cc   |  2 +-
>  3 files changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc
> index 7987c406ab4..1410dd96551 100644
> --- a/gdbserver/regcache.cc
> +++ b/gdbserver/regcache.cc
> @@ -43,7 +43,7 @@ get_thread_regcache (struct thread_info *thread, int fetch)
>  
>        gdb_assert (proc->tdesc != NULL);
>  
> -      regcache = new_register_cache (proc->tdesc);
> +      regcache = new struct regcache (proc->tdesc);
>        set_thread_regcache_data (thread, regcache);
>      }
>  
> @@ -151,15 +151,10 @@ regcache::initialize (const target_desc *tdesc,
>  
>  #ifndef IN_PROCESS_AGENT
>  
> -struct regcache *
> -new_register_cache (const struct target_desc *tdesc)
> +regcache::regcache (const target_desc *tdesc)
>  {
> -  struct regcache *regcache = new struct regcache;
> -
>    gdb_assert (tdesc->registers_size != 0);
> -  regcache->initialize (tdesc, nullptr);
> -
> -  return regcache;
> +  initialize (tdesc, nullptr);
>  }
>  
>  void
> diff --git a/gdbserver/regcache.h b/gdbserver/regcache.h
> index 15b7e2b4dff..0002726ed00 100644
> --- a/gdbserver/regcache.h
> +++ b/gdbserver/regcache.h
> @@ -44,6 +44,10 @@ struct regcache : public reg_buffer_common
>  #ifndef IN_PROCESS_AGENT
>    /* One of REG_UNAVAILABLE or REG_VALID.  */
>    unsigned char *register_status = nullptr;
> +
> +  /* Constructors.  */
> +  regcache () = default;
> +  regcache (const target_desc *tdesc);

Ok, so with this patch I understood better the two modes regcache can
operate, it can allocate its own memory or use a buffer passed to the
initialize method.  I think my previous suggestion still stands.  You
could have:

  regcache (const target_desc *tdesc,  unsigned char *regbuf);
  regcache (const target_desc *tdesc);

... former using the passed-in buffer and the latter allocating its own
buffer.  I don't think we would need a default constructor then (and we
should use DISALLOW_COPY_AND_ASSIGN).

Simon
  

Patch

diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc
index 7987c406ab4..1410dd96551 100644
--- a/gdbserver/regcache.cc
+++ b/gdbserver/regcache.cc
@@ -43,7 +43,7 @@  get_thread_regcache (struct thread_info *thread, int fetch)
 
       gdb_assert (proc->tdesc != NULL);
 
-      regcache = new_register_cache (proc->tdesc);
+      regcache = new struct regcache (proc->tdesc);
       set_thread_regcache_data (thread, regcache);
     }
 
@@ -151,15 +151,10 @@  regcache::initialize (const target_desc *tdesc,
 
 #ifndef IN_PROCESS_AGENT
 
-struct regcache *
-new_register_cache (const struct target_desc *tdesc)
+regcache::regcache (const target_desc *tdesc)
 {
-  struct regcache *regcache = new struct regcache;
-
   gdb_assert (tdesc->registers_size != 0);
-  regcache->initialize (tdesc, nullptr);
-
-  return regcache;
+  initialize (tdesc, nullptr);
 }
 
 void
diff --git a/gdbserver/regcache.h b/gdbserver/regcache.h
index 15b7e2b4dff..0002726ed00 100644
--- a/gdbserver/regcache.h
+++ b/gdbserver/regcache.h
@@ -44,6 +44,10 @@  struct regcache : public reg_buffer_common
 #ifndef IN_PROCESS_AGENT
   /* One of REG_UNAVAILABLE or REG_VALID.  */
   unsigned char *register_status = nullptr;
+
+  /* Constructors.  */
+  regcache () = default;
+  regcache (const target_desc *tdesc);
 #endif
 
   /* Init the regcache data.  */
@@ -64,10 +68,6 @@  struct regcache : public reg_buffer_common
 
 void regcache_cpy (struct regcache *dst, struct regcache *src);
 
-/* Create a new register cache for INFERIOR.  */
-
-struct regcache *new_register_cache (const struct target_desc *tdesc);
-
 struct regcache *get_thread_regcache (struct thread_info *thread, int fetch);
 
 /* Release all memory associated with the register cache for INFERIOR.  */
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 949849b63a2..f85d7ed9cf5 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -4238,7 +4238,7 @@  process_serial_event (void)
       if (cs.current_traceframe >= 0)
 	{
 	  struct regcache *regcache
-	    = new_register_cache (current_target_desc ());
+	    = new struct regcache (current_target_desc ());
 
 	  if (fetch_traceframe_registers (cs.current_traceframe,
 					  regcache, -1) == 0)