[1/9] Change remote-sim.c to use type-safe registry

Message ID 20190710153947.25721-2-tromey@adacore.com
State New
Headers show
Series
  • Use the type-safe registry in more cases
Related show

Commit Message

Tom Tromey July 10, 2019, 3:39 p.m.
This changes remote-sim.c to use the type-safe registry.

gdb/ChangeLog
2019-07-10  Tom Tromey  <tromey@adacore.com>

	* remote-sim.c (struct sim_inferior_data): Add initializers and
	destructor.
	(sim_inferior_data_key): Change type.  Move lower.
	(check_for_duplicate_sim_descriptor): Update.
	(get_sim_inferior_data): Use new.  Update.
	(~sim_inferior_data_cleanup): Rename from
	sim_inferior_data_cleanup.  Simplify.
	(gdbsim_close_inferior, simulator_command)
	(sim_command_completer, _initialize_remote_sim): Update.
---
 gdb/ChangeLog    | 12 ++++++++++
 gdb/remote-sim.c | 57 +++++++++++++++---------------------------------
 2 files changed, 30 insertions(+), 39 deletions(-)

-- 
2.20.1

Comments

Simon Marchi July 10, 2019, 4:59 p.m. | #1
Hi Tom,

Just one nit:

> diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c

> index a22a59ed399..9030ead562d 100644

> --- a/gdb/remote-sim.c

> +++ b/gdb/remote-sim.c

> @@ -132,15 +132,15 @@ struct gdbsim_target final

>  

>  static struct gdbsim_target gdbsim_ops;

>  

> -static const struct inferior_data *sim_inferior_data_key;

> -

>  /* Simulator-specific, per-inferior state.  */

>  struct sim_inferior_data {

> +  ~sim_inferior_data ();

> +

>    /* Flag which indicates whether or not the program has been loaded.  */

> -  int program_loaded;

> +  int program_loaded = 0;

>  

>    /* Simulator descriptor for this inferior.  */

> -  SIM_DESC gdbsim_desc;

> +  SIM_DESC gdbsim_desc = nullptr;

>  

>    /* This is the ptid we use for this particular simulator instance.  Its

>       value is somewhat arbitrary, as the simulator target don't have a

> @@ -150,12 +150,14 @@ struct sim_inferior_data {

>    ptid_t remote_sim_ptid;


It probably doesn't matter because it's reassigned after sim_inferior_data,
is allocated, but this field should probably be initialized too, to be consistent
with the other fields.  ptid_t doesn't initialize its fields by default (to remain
a POD), so its initial content would be some garbage.

Otherwise, LGTM.

Simon
Tom Tromey July 10, 2019, 5:03 p.m. | #2
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:


Simon> It probably doesn't matter because it's reassigned after sim_inferior_data,
Simon> is allocated, but this field should probably be initialized too, to be consistent
Simon> with the other fields.  ptid_t doesn't initialize its fields by default (to remain
Simon> a POD), so its initial content would be some garbage.

Thanks for catching that.  I forgot about this aspect of ptid_t.  I've
made the change locally.

Tom

Patch

diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index a22a59ed399..9030ead562d 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -132,15 +132,15 @@  struct gdbsim_target final
 
 static struct gdbsim_target gdbsim_ops;
 
-static const struct inferior_data *sim_inferior_data_key;
-
 /* Simulator-specific, per-inferior state.  */
 struct sim_inferior_data {
+  ~sim_inferior_data ();
+
   /* Flag which indicates whether or not the program has been loaded.  */
-  int program_loaded;
+  int program_loaded = 0;
 
   /* Simulator descriptor for this inferior.  */
-  SIM_DESC gdbsim_desc;
+  SIM_DESC gdbsim_desc = nullptr;
 
   /* This is the ptid we use for this particular simulator instance.  Its
      value is somewhat arbitrary, as the simulator target don't have a
@@ -150,12 +150,14 @@  struct sim_inferior_data {
   ptid_t remote_sim_ptid;
 
   /* Signal with which to resume.  */
-  enum gdb_signal resume_siggnal;
+  enum gdb_signal resume_siggnal = GDB_SIGNAL_0;
 
   /* Flag which indicates whether resume should step or not.  */
-  int resume_step;
+  int resume_step = 0;
 };
 
+static inferior_key<sim_inferior_data> sim_inferior_data_key;
+
 /* Flag indicating the "open" status of this module.  It's set to 1
    in gdbsim_open() and 0 in gdbsim_close().  */
 static int gdbsim_is_open = 0;
@@ -186,8 +188,7 @@  check_for_duplicate_sim_descriptor (struct inferior *inf, void *arg)
   struct sim_inferior_data *sim_data;
   SIM_DESC new_sim_desc = (SIM_DESC) arg;
 
-  sim_data = ((struct sim_inferior_data *)
-	      inferior_data (inf, sim_inferior_data_key));
+  sim_data = sim_inferior_data_key.get (inf);
 
   return (sim_data != NULL && sim_data->gdbsim_desc == new_sim_desc);
 }
@@ -204,8 +205,7 @@  static struct sim_inferior_data *
 get_sim_inferior_data (struct inferior *inf, int sim_instance_needed)
 {
   SIM_DESC sim_desc = NULL;
-  struct sim_inferior_data *sim_data
-    = (struct sim_inferior_data *) inferior_data (inf, sim_inferior_data_key);
+  struct sim_inferior_data *sim_data = sim_inferior_data_key.get (inf);
 
   /* Try to allocate a new sim instance, if needed.  We do this ahead of
      a potential allocation of a sim_inferior_data struct in order to
@@ -240,18 +240,14 @@  get_sim_inferior_data (struct inferior *inf, int sim_instance_needed)
 
   if (sim_data == NULL)
     {
-      sim_data = XCNEW(struct sim_inferior_data);
-      set_inferior_data (inf, sim_inferior_data_key, sim_data);
+      sim_data = sim_inferior_data_key.emplace (inf);
 
       /* Allocate a ptid for this inferior.  */
       sim_data->remote_sim_ptid = ptid_t (next_pid, 0, next_pid);
       next_pid++;
 
       /* Initialize the other instance variables.  */
-      sim_data->program_loaded = 0;
       sim_data->gdbsim_desc = sim_desc;
-      sim_data->resume_siggnal = GDB_SIGNAL_0;
-      sim_data->resume_step = 0;
     }
   else if (sim_desc)
     {
@@ -287,20 +283,10 @@  get_sim_inferior_data_by_ptid (ptid_t ptid, int sim_instance_needed)
 
 /* Free the per-inferior simulator data.  */
 
-static void
-sim_inferior_data_cleanup (struct inferior *inf, void *data)
+sim_inferior_data::~sim_inferior_data ()
 {
-  struct sim_inferior_data *sim_data = (struct sim_inferior_data *) data;
-
-  if (sim_data != NULL)
-    {
-      if (sim_data->gdbsim_desc)
-	{
-	  sim_close (sim_data->gdbsim_desc, 0);
-	  sim_data->gdbsim_desc = NULL;
-	}
-      xfree (sim_data);
-    }
+  if (gdbsim_desc)
+    sim_close (gdbsim_desc, 0);
 }
 
 static void
@@ -789,14 +775,12 @@  gdbsim_target_open (const char *args, int from_tty)
 static int
 gdbsim_close_inferior (struct inferior *inf, void *arg)
 {
-  struct sim_inferior_data *sim_data
-    = (struct sim_inferior_data *) inferior_data (inf, sim_inferior_data_key);
+  struct sim_inferior_data *sim_data = sim_inferior_data_key.get (inf);
   if (sim_data != NULL)
     {
       ptid_t ptid = sim_data->remote_sim_ptid;
 
-      sim_inferior_data_cleanup (inf, sim_data);
-      set_inferior_data (inf, sim_inferior_data_key, NULL);
+      sim_inferior_data_key.clear (inf);
 
       /* Having a ptid allocated and stored in remote_sim_ptid does
 	 not mean that a corresponding inferior was ever created.
@@ -1193,8 +1177,7 @@  simulator_command (const char *args, int from_tty)
      thus allocating memory that would not be garbage collected until
      the ultimate destruction of the associated inferior.  */
 
-  sim_data  = ((struct sim_inferior_data *)
-	       inferior_data (current_inferior (), sim_inferior_data_key));
+  sim_data  = sim_inferior_data_key.get (current_inferior ());
   if (sim_data == NULL || sim_data->gdbsim_desc == NULL)
     {
 
@@ -1225,8 +1208,7 @@  sim_command_completer (struct cmd_list_element *ignore,
 {
   struct sim_inferior_data *sim_data;
 
-  sim_data = ((struct sim_inferior_data *)
-	      inferior_data (current_inferior (), sim_inferior_data_key));
+  sim_data = sim_inferior_data_key.get (current_inferior ());
   if (sim_data == NULL || sim_data->gdbsim_desc == NULL)
     return;
 
@@ -1324,7 +1306,4 @@  _initialize_remote_sim (void)
   c = add_com ("sim", class_obscure, simulator_command,
 	       _("Send a command to the simulator."));
   set_cmd_completer (c, sim_command_completer);
-
-  sim_inferior_data_key
-    = register_inferior_data_with_cleanup (NULL, sim_inferior_data_cleanup);
 }