Patchwork [05/10] remote: remote_arch_state pointers -> remote_arch_state objects

login
register
mail settings
Submitter Pedro Alves
Date May 16, 2018, 2:18 p.m.
Message ID <20180516141830.16859-6-palves@redhat.com>
Download mbox | patch
Permalink /patch/27287/
State New
Headers show

Comments

Pedro Alves - May 16, 2018, 2:18 p.m.
The previous patch made the map store pointers to remote_arch_state
instead of objects directly, simply because struct remote_arch_state
is still incomplete where struct remote_state is declared.  This patch
thus moves the remote_arch_state declaration higher up in the file,
and makes the map store remote_arch_state objects directly instead of
pointers to objects.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* remote.c (struct packet_reg, struct remote_arch_state):
	Move higher up in the file.
	(remote_state) <m_arch_states>: Store remote_arch_state values
	instead of remote_arch_state pointers.
	(remote_state::get_remote_arch_state): Adjust.
---
 gdb/remote.c | 93 ++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 49 insertions(+), 44 deletions(-)
Simon Marchi - May 18, 2018, 9:05 p.m.
On 2018-05-16 10:18 AM, Pedro Alves wrote:
> The previous patch made the map store pointers to remote_arch_state
> instead of objects directly, simply because struct remote_arch_state
> is still incomplete where struct remote_state is declared.  This patch
> thus moves the remote_arch_state declaration higher up in the file,
> and makes the map store remote_arch_state objects directly instead of
> pointers to objects.

LGTM.

Simon
Simon Marchi - May 18, 2018, 9:09 p.m.
On 2018-05-18 05:05 PM, Simon Marchi wrote:
> On 2018-05-16 10:18 AM, Pedro Alves wrote:
>> The previous patch made the map store pointers to remote_arch_state
>> instead of objects directly, simply because struct remote_arch_state
>> is still incomplete where struct remote_state is declared.  This patch
>> thus moves the remote_arch_state declaration higher up in the file,
>> and makes the map store remote_arch_state objects directly instead of
>> pointers to objects.
> 
> LGTM.
> 
> Simon
> 

Oops, spoke a bit too soon.  It fails to build with g++ 5.4.0 (Ubuntu 16.04).
It doesn't like the "this->m_arch_states.emplace (gdbarch, gdbarch)".  With
g++ 7.1.0 it builds fine.

/home/emaisin/src/binutils-gdb/gdb/remote.c:962:61:   required from here
/usr/include/c++/5/ext/new_allocator.h:120:4: error: no matching function for call to ‘std::pair<gdbarch* const, remote_arch_state>::pair(gdbarch*&, gdbarch*&)’
  { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); }
    ^

Simon

Patch

diff --git a/gdb/remote.c b/gdb/remote.c
index 7903cb6344..8e1baf0856 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -603,6 +603,44 @@  struct readahead_cache
   ULONGEST miss_count = 0;
 };
 
+/* Description of the remote protocol for a given architecture.  */
+
+struct packet_reg
+{
+  long offset; /* Offset into G packet.  */
+  long regnum; /* GDB's internal register number.  */
+  LONGEST pnum; /* Remote protocol register number.  */
+  int in_g_packet; /* Always part of G packet.  */
+  /* long size in bytes;  == register_size (target_gdbarch (), regnum);
+     at present.  */
+  /* char *name; == gdbarch_register_name (target_gdbarch (), regnum);
+     at present.  */
+};
+
+struct remote_arch_state
+{
+  explicit remote_arch_state (struct gdbarch *gdbarch);
+
+  /* Description of the remote protocol registers.  */
+  long sizeof_g_packet;
+
+  /* Description of the remote protocol registers indexed by REGNUM
+     (making an array gdbarch_num_regs in size).  */
+  std::unique_ptr<packet_reg[]> regs;
+
+  /* This is the size (in chars) of the first response to the ``g''
+     packet.  It is used as a heuristic when determining the maximum
+     size of memory-read and memory-write packets.  A target will
+     typically only reserve a buffer large enough to hold the ``g''
+     packet.  The size does not include packet overhead (headers and
+     trailers).  */
+  long actual_register_packet_size;
+
+  /* This is the maximum size (in chars) of a non read/write packet.
+     It is also used as a cap on the size of read/write packets.  */
+  long remote_packet_size;
+};
+
 /* Description of the remote protocol state for the currently
    connected target.  This is per-target state, and independent of the
    selected architecture.  */
@@ -749,8 +787,7 @@  private:
   /* Mapping of remote protocol data for each gdbarch.  Usually there
      is only one entry here, though we may see more with stubs that
      support multi-process.  */
-  std::unordered_map<struct gdbarch *,
-		     std::unique_ptr<struct remote_arch_state>>
+  std::unordered_map<struct gdbarch *, remote_arch_state>
     m_arch_states;
 };
 
@@ -817,44 +854,6 @@  get_remote_state_raw (void)
   return remote_state;
 }
 
-/* Description of the remote protocol for a given architecture.  */
-
-struct packet_reg
-{
-  long offset; /* Offset into G packet.  */
-  long regnum; /* GDB's internal register number.  */
-  LONGEST pnum; /* Remote protocol register number.  */
-  int in_g_packet; /* Always part of G packet.  */
-  /* long size in bytes;  == register_size (target_gdbarch (), regnum);
-     at present.  */
-  /* char *name; == gdbarch_register_name (target_gdbarch (), regnum);
-     at present.  */
-};
-
-struct remote_arch_state
-{
-  explicit remote_arch_state (struct gdbarch *gdbarch);
-
-  /* Description of the remote protocol registers.  */
-  long sizeof_g_packet;
-
-  /* Description of the remote protocol registers indexed by REGNUM
-     (making an array gdbarch_num_regs in size).  */
-  std::unique_ptr<packet_reg[]> regs;
-
-  /* This is the size (in chars) of the first response to the ``g''
-     packet.  It is used as a heuristic when determining the maximum
-     size of memory-read and memory-write packets.  A target will
-     typically only reserve a buffer large enough to hold the ``g''
-     packet.  The size does not include packet overhead (headers and
-     trailers).  */
-  long actual_register_packet_size;
-
-  /* This is the maximum size (in chars) of a non read/write packet.
-     It is also used as a cap on the size of read/write packets.  */
-  long remote_packet_size;
-};
-
 /* Utility: generate error from an incoming stub packet.  */
 static void
 trace_error (char *buf)
@@ -955,10 +954,13 @@  remote_get_noisy_reply ()
 struct remote_arch_state *
 remote_state::get_remote_arch_state (struct gdbarch *gdbarch)
 {
-  auto &rsa = this->m_arch_states[gdbarch];
-  if (rsa == nullptr)
+  remote_arch_state *rsa;
+
+  auto it = this->m_arch_states.find (gdbarch);
+  if (it == this->m_arch_states.end ())
     {
-      rsa.reset (new remote_arch_state (gdbarch));
+      auto p = this->m_arch_states.emplace (gdbarch, gdbarch);
+      rsa = &p.first->second;
 
       /* Make sure that the packet buffer is plenty big enough for
 	 this architecture.  */
@@ -968,7 +970,10 @@  remote_state::get_remote_arch_state (struct gdbarch *gdbarch)
 	  this->buf = (char *) xrealloc (this->buf, this->buf_size);
 	}
     }
-  return rsa.get ();
+  else
+    rsa = &it->second;
+
+  return rsa;
 }
 
 /* Fetch the global remote target state.  */