[10/26] gdbserver: convert registers_to_string into regcache::registers_to_string

Message ID 5ff95fd684084ed53879f1c2c5f96be1a9d4a59a.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 free `registers_to_string` function into a method of
the regcache struct.
---
 gdbserver/regcache.cc | 12 +++++-------
 gdbserver/regcache.h  |  9 ++++-----
 gdbserver/server.cc   |  4 ++--
 3 files changed, 11 insertions(+), 14 deletions(-)
  

Comments

Simon Marchi Dec. 21, 2023, 9:13 p.m. UTC | #1
On 2/28/23 06:28, Tankut Baris Aktemur via Gdb-patches wrote:
> Convert the free `registers_to_string` function into a method of
> the regcache struct.

I'm not strongly opposed to this change, but I think that it works just
as well as a free function.  Not everything working on a regcache needs
to be a method of regcache.  If you imagine that all the members of
regcache are private, and it offers an interface to access the register
contents and statuses, then registers_to_string can very well be
implemented using that public interface.

Simon
  
Simon Marchi Dec. 21, 2023, 9:19 p.m. UTC | #2
On 12/21/23 16:13, Simon Marchi wrote:
> On 2/28/23 06:28, Tankut Baris Aktemur via Gdb-patches wrote:
>> Convert the free `registers_to_string` function into a method of
>> the regcache struct.
> 
> I'm not strongly opposed to this change, but I think that it works just
> as well as a free function.  Not everything working on a regcache needs
> to be a method of regcache.  If you imagine that all the members of
> regcache are private, and it offers an interface to access the register
> contents and statuses, then registers_to_string can very well be
> implemented using that public interface.

To clarify, I think that this functionality might belong more in the
remote-utils.cc's area of responsibility, since it's serializing the
regcache's content specifically for the remote protocol.

Simon
  

Patch

diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc
index 1db78951cc2..92dc6fc921c 100644
--- a/gdbserver/regcache.cc
+++ b/gdbserver/regcache.cc
@@ -214,16 +214,14 @@  find_register_by_number (const struct target_desc *tdesc, int n)
 #ifndef IN_PROCESS_AGENT
 
 void
-registers_to_string (struct regcache *regcache, char *buf)
+regcache::registers_to_string (char *buf)
 {
-  unsigned char *registers = regcache->registers;
-  const struct target_desc *tdesc = regcache->tdesc;
-
+  unsigned char *regs = registers;
   for (int i = 0; i < tdesc->reg_defs.size (); ++i)
     {
-      if (regcache->register_status[i] == REG_VALID)
+      if (register_status[i] == REG_VALID)
 	{
-	  bin2hex (registers, buf, register_size (tdesc, i));
+	  bin2hex (regs, buf, register_size (tdesc, i));
 	  buf += register_size (tdesc, i) * 2;
 	}
       else
@@ -231,7 +229,7 @@  registers_to_string (struct regcache *regcache, char *buf)
 	  memset (buf, 'x', register_size (tdesc, i) * 2);
 	  buf += register_size (tdesc, i) * 2;
 	}
-      registers += register_size (tdesc, i);
+      regs += register_size (tdesc, i);
     }
   *buf = '\0';
 }
diff --git a/gdbserver/regcache.h b/gdbserver/regcache.h
index b4a9d8864ae..bed3151dcad 100644
--- a/gdbserver/regcache.h
+++ b/gdbserver/regcache.h
@@ -84,6 +84,10 @@  struct regcache : public reg_buffer_common
 
   /* Discard the cache without storing the registers to the target.  */
   void discard ();
+
+  /* Convert all registers to a string in the currently specified remote
+     format.  */
+  void registers_to_string (char *buf);
 };
 
 regcache *get_thread_regcache (thread_info *thread, bool fetch = true);
@@ -106,11 +110,6 @@  void regcache_invalidate (void);
 
 void regcache_release (void);
 
-/* Convert all registers to a string in the currently specified remote
-   format.  */
-
-void registers_to_string (struct regcache *regcache, char *buf);
-
 /* Convert a string to register values and fill our register cache.  */
 
 void registers_from_string (struct regcache *regcache, char *buf);
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 521f13c07d7..f540cc86f7b 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -4241,7 +4241,7 @@  process_serial_event (void)
 
 	  if (fetch_traceframe_registers (cs.current_traceframe,
 					  &a_regcache, -1) == 0)
-	    registers_to_string (&a_regcache, cs.own_buf);
+	    a_regcache.registers_to_string (cs.own_buf);
 	  else
 	    write_enn (cs.own_buf);
 	}
@@ -4254,7 +4254,7 @@  process_serial_event (void)
 	  else
 	    {
 	      regcache = get_thread_regcache (current_thread);
-	      registers_to_string (regcache, cs.own_buf);
+	      regcache->registers_to_string (cs.own_buf);
 	    }
 	}
       break;