Discussion:
[PATCH] Remove a VEC from remote.c
Tom Tromey
2018-11-04 16:06:49 UTC
Permalink
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 <***@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
Simon Marchi
2018-11-09 22:13:37 UTC
Permalink
Post by 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.
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.
Post by Tom Tromey
+ 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.

S
Tom Tromey
2018-11-09 23:05:45 UTC
Permalink
Post by Tom Tromey
+ 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

Loading...