[PATCH] Remove a VEC from remote.c

[PATCH] Remove a VEC from remote.c

[Tom Tromey]

This removes the VEC from remote_g_packet_data, replacing it with a
std::vector.  This is a bit odd in that this object is never
destroyed, and is obstack-allocated.  I believe a gdbarch is never
destroyed, so this seemed ok.

Tested by the buildbot.

gdb/ChangeLog
2018-11-04  Tom Tromey  <tom@tromey.com>

	* remote.c (remote_g_packet_guess_s): Remove typedef and DEF_VEC.
	(struct remote_g_packet_data): Derive from allocate_on_obstack.
	<guesses>: Now a std::vector.
	(remote_g_packet_data_init, register_remote_g_packet_guess):
	Update.
	(remote_read_description_p): Update.  Return bool.
	(remote_target::read_description): Update.
---
 gdb/ChangeLog | 10 ++++++++++
 gdb/remote.c  | 44 ++++++++++++++++----------------------------
 2 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index c53553af5b..f7a398ec95 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1030,7 +1030,7 @@ static ptid_t read_ptid (const char *buf, const char **obuf);
 
 static void remote_async_inferior_event_handler (gdb_client_data);
 
-static int remote_read_description_p (struct target_ops *target);
+static bool remote_read_description_p (struct target_ops *target);
 
 static void remote_console_output (char *msg);
 
@@ -11620,12 +11620,10 @@ struct remote_g_packet_guess
   int bytes;
   const struct target_desc *tdesc;
 };
-typedef struct remote_g_packet_guess remote_g_packet_guess_s;
-DEF_VEC_O(remote_g_packet_guess_s);
 
-struct remote_g_packet_data
+struct remote_g_packet_data : public allocate_on_obstack
 {
-  VEC(remote_g_packet_guess_s) *guesses;
+  std::vector<remote_g_packet_guess> guesses;
 };
 
 static struct gdbarch_data *remote_g_packet_data_handle;
@@ -11633,7 +11631,7 @@ static struct gdbarch_data *remote_g_packet_data_handle;
 static void *
 remote_g_packet_data_init (struct obstack *obstack)
 {
-  return OBSTACK_ZALLOC (obstack, struct remote_g_packet_data);
+  return new (obstack) remote_g_packet_data;
 }
 
 void
@@ -11643,38 +11641,32 @@ register_remote_g_packet_guess (struct gdbarch *gdbarch, int bytes,
   struct remote_g_packet_data *data
     = ((struct remote_g_packet_data *)
        gdbarch_data (gdbarch, remote_g_packet_data_handle));
-  struct remote_g_packet_guess new_guess, *guess;
-  int ix;
+  struct remote_g_packet_guess new_guess;
 
   gdb_assert (tdesc != NULL);
 
-  for (ix = 0;
-       VEC_iterate (remote_g_packet_guess_s, data->guesses, ix, guess);
-       ix++)
-    if (guess->bytes == bytes)
+  for (const remote_g_packet_guess &guess : data->guesses)
+    if (guess.bytes == bytes)
       internal_error (__FILE__, __LINE__,
 		      _("Duplicate g packet description added for size %d"),
 		      bytes);
 
   new_guess.bytes = bytes;
   new_guess.tdesc = tdesc;
-  VEC_safe_push (remote_g_packet_guess_s, data->guesses, &new_guess);
+  data->guesses.push_back (new_guess);
 }
 
-/* Return 1 if remote_read_description would do anything on this target
-   and architecture, 0 otherwise.  */
+/* Return true if remote_read_description would do anything on this target
+   and architecture, false otherwise.  */
 
-static int
+static bool
 remote_read_description_p (struct target_ops *target)
 {
   struct remote_g_packet_data *data
     = ((struct remote_g_packet_data *)
        gdbarch_data (target_gdbarch (), remote_g_packet_data_handle));
 
-  if (!VEC_empty (remote_g_packet_guess_s, data->guesses))
-    return 1;
-
-  return 0;
+  return !data->guesses.empty ();
 }
 
 const struct target_desc *
@@ -11689,17 +11681,13 @@ remote_target::read_description ()
   if (!target_has_execution || inferior_ptid == null_ptid)
     return beneath ()->read_description ();
 
-  if (!VEC_empty (remote_g_packet_guess_s, data->guesses))
+  if (!data->guesses.empty ())
     {
-      struct remote_g_packet_guess *guess;
-      int ix;
       int bytes = send_g_packet ();
 
-      for (ix = 0;
-	   VEC_iterate (remote_g_packet_guess_s, data->guesses, ix, guess);
-	   ix++)
-	if (guess->bytes == bytes)
-	  return guess->tdesc;
+      for (const remote_g_packet_guess &guess : data->guesses)
+	if (guess.bytes == bytes)
+	  return guess.tdesc;
 
       /* We discard the g packet.  A minor optimization would be to
 	 hold on to it, and fill the register cache once we have selected
-- 
2.17.1

Re: [PATCH] Remove a VEC from remote.c

[Simon Marchi]

On 2018-11-04 11:06 a.m., Tom Tromey wrote:
> This removes the VEC from remote_g_packet_data, replacing it with a
> std::vector.  This is a bit odd in that this object is never
> destroyed, and is obstack-allocated.  I believe a gdbarch is never
> destroyed, so this seemed ok.

This seems ok to me.  If gdbarch were destroyable, we would need a new
"cleanup" hook to call ~remote_g_packet_data.  But we would need such a
hook even without your patch, to free the VEC, which it itself not allocated
on the gdbarch obstack.

Tiny nit:

> +  data->guesses.push_back (new_guess);

I always prefer having a constructor and using

  data->guesses.emplace_back (bytes, tdesc);

since it makes it impossible to have uninitialized fields by mistake.

Simon

Re: [PATCH] Remove a VEC from remote.c

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

>> +  data->guesses.push_back (new_guess);

Simon> I always prefer having a constructor and using
Simon> data-> guesses.emplace_back (bytes, tdesc);
Simon> since it makes it impossible to have uninitialized fields by mistake.

I'm checking it in with this change.

Tom