[PATCH 0/4] Remove some uses of iterate_over_inferiors

[PATCH 0/4] Remove some uses of iterate_over_inferiors

[Tom Tromey]

This series removes some uses of iterate_over_inferiors in favor of
foreach-style iteration.  I find the latter simpler both to debug and
to read.

There are more instances of iterate_over_inferiors, and other
iterate_over_* functions -- this series is just a starter to see
whether others agree, so let me know what you think.

Tested on x86-64 Fedora by the buildbot.

Tom

[PATCH 1/4] Use foreach in gdbpy_inferiors

[Tom Tromey]

This changes gdbpy_inferiors to use foreach rather than a callback
function.

gdb/ChangeLog
2019-09-19  Tom Tromey  <tom@tromey.com>

	* python/py-inferior.c (build_inferior_list): Remove.
	(gdbpy_inferiors): Use foreach.
---
 gdb/ChangeLog            |  5 +++++
 gdb/python/py-inferior.c | 24 ++++++++++--------------
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 28882221a90..ecb95444a36 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -462,18 +462,6 @@ infpy_get_progspace (PyObject *self, void *closure)
   return pspace_to_pspace_object (pspace).release ();
 }
 
-static int
-build_inferior_list (struct inferior *inf, void *arg)
-{
-  PyObject *list = (PyObject *) arg;
-  gdbpy_ref<inferior_object> inferior = inferior_to_inferior_object (inf);
-
-  if (inferior == NULL)
-    return 0;
-
-  return PyList_Append (list, (PyObject *) inferior.get ()) ? 1 : 0;
-}
-
 /* Implementation of gdb.inferiors () -> (gdb.Inferior, ...).
    Returns a tuple of all inferiors.  */
 PyObject *
@@ -483,8 +471,16 @@ gdbpy_inferiors (PyObject *unused, PyObject *unused2)
   if (list == NULL)
     return NULL;
 
-  if (iterate_over_inferiors (build_inferior_list, list.get ()))
-    return NULL;
+  for (inferior *inf : all_inferiors_safe ())
+    {
+      gdbpy_ref<inferior_object> inferior = inferior_to_inferior_object (inf);
+
+      if (inferior == NULL)
+	continue;
+
+      if (PyList_Append (list.get (), (PyObject *) inferior.get ()) == -1)
+	return NULL;
+    }
 
   return PyList_AsTuple (list.get ());
 }
-- 
2.17.2

[PATCH 3/4] Don't use iterate_over_inferiors in target.c

[Tom Tromey]

This changes target.c to use C++ iterators rather than
iterate_over_inferiors.

gdb/ChangeLog
2019-09-19  Tom Tromey  <tom@tromey.com>

	* target.c (dispose_inferiors): Rename from dispose_inferior.
	Change type.
	(target_preopen): Update.
---
 gdb/ChangeLog |  6 ++++++
 gdb/target.c  | 43 ++++++++++++++++++++++---------------------
 2 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/gdb/target.c b/gdb/target.c
index 78bdfeb49a4..c414c022c75 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1951,31 +1951,32 @@ target_pre_inferior (int from_tty)
   agent_capability_invalidate ();
 }
 
-/* Callback for iterate_over_inferiors.  Gets rid of the given
-   inferior.  */
+/* Gets rid of all inferiors.  */
 
-static int
-dispose_inferior (struct inferior *inf, void *args)
+static void
+dispose_inferiors ()
 {
-  /* Not all killed inferiors can, or will ever be, removed from the
-     inferior list.  Killed inferiors clearly don't need to be killed
-     again, so, we're done.  */
-  if (inf->pid == 0)
-    return 0;
-
-  thread_info *thread = any_thread_of_inferior (inf);
-  if (thread != NULL)
+  for (inferior *inf : all_inferiors_safe ())
     {
-      switch_to_thread (thread);
+      /* Not all killed inferiors can, or will ever be, removed from
+	 the inferior list.  Killed inferiors clearly don't need to be
+	 killed again, so, we're done.  */
+      if (inf->pid == 0)
+	continue;
 
-      /* Core inferiors actually should be detached, not killed.  */
-      if (target_has_execution)
-	target_kill ();
-      else
-	target_detach (inf, 0);
-    }
+      thread_info *thread = any_thread_of_inferior (inf);
+      if (thread != NULL)
+	{
+	  switch_to_thread (thread);
 
-  return 0;
+	  /* Core inferiors actually should be detached, not
+	     killed.  */
+	  if (target_has_execution)
+	    target_kill ();
+	  else
+	    target_detach (inf, 0);
+	}
+    }
 }
 
 /* This is to be called by the open routine before it does
@@ -1991,7 +1992,7 @@ target_preopen (int from_tty)
       if (!from_tty
 	  || !have_live_inferiors ()
 	  || query (_("A program is being debugged already.  Kill it? ")))
-	iterate_over_inferiors (dispose_inferior, NULL);
+	dispose_inferiors ();
       else
 	error (_("Program not killed."));
     }
-- 
2.17.2

[PATCH 4/4] Remove a use of iterate_over_threads from mi-main.c

[Tom Tromey]

This removes one use of iterate_over_threads from mi-main.c.

gdb/ChangeLog
2019-09-19  Tom Tromey  <tom@tromey.com>

	* mi/mi-main.c (run_inferiors): Rename from run_one_inferior.
	Change type.
	(mi_cmd_exec_run): Update.  Use bool.
---
 gdb/ChangeLog    |  6 +++++
 gdb/mi/mi-main.c | 57 +++++++++++++++++++++++-------------------------
 2 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 2ece3602056..30cc79ce27c 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -391,44 +391,41 @@ mi_cmd_exec_interrupt (const char *command, char **argv, int argc)
     }
 }
 
-/* Callback for iterate_over_inferiors which starts the execution
-   of the given inferior.
+/* Start the execution of all inferiors.
 
-   ARG is a pointer to an integer whose value, if non-zero, indicates
-   that the program should be stopped when reaching the main subprogram
-   (similar to what the CLI "start" command does).  */
+   RUN_CMD is the run command to use, either "run" or "start".  */
 
-static int
-run_one_inferior (struct inferior *inf, void *arg)
+static void
+run_inferiors (const char *run_cmd)
 {
-  int start_p = *(int *) arg;
-  const char *run_cmd = start_p ? "start" : "run";
-  struct target_ops *run_target = find_run_target ();
-  int async_p = mi_async && run_target->can_async_p ();
-
-  if (inf->pid != 0)
+  for (inferior *inf : all_inferiors_safe ())
     {
-      thread_info *tp = any_thread_of_inferior (inf);
-      if (tp == NULL)
-	error (_("Inferior has no threads."));
+      struct target_ops *run_target = find_run_target ();
+      int async_p = mi_async && run_target->can_async_p ();
 
-      switch_to_thread (tp);
-    }
-  else
-    {
-      set_current_inferior (inf);
-      switch_to_no_thread ();
-      set_current_program_space (inf->pspace);
+      if (inf->pid != 0)
+	{
+	  thread_info *tp = any_thread_of_inferior (inf);
+	  if (tp == NULL)
+	    error (_("Inferior has no threads."));
+
+	  switch_to_thread (tp);
+	}
+      else
+	{
+	  set_current_inferior (inf);
+	  switch_to_no_thread ();
+	  set_current_program_space (inf->pspace);
+	}
+      mi_execute_cli_command (run_cmd, async_p,
+			      async_p ? "&" : NULL);
     }
-  mi_execute_cli_command (run_cmd, async_p,
-			  async_p ? "&" : NULL);
-  return 0;
 }
 
 void
 mi_cmd_exec_run (const char *command, char **argv, int argc)
 {
-  int start_p = 0;
+  bool start_p = false;
 
   /* Parse the command options.  */
   enum opt
@@ -453,7 +450,7 @@ mi_cmd_exec_run (const char *command, char **argv, int argc)
       switch ((enum opt) opt)
 	{
 	case START_OPT:
-	  start_p = 1;
+	  start_p = true;
 	  break;
 	}
     }
@@ -463,15 +460,15 @@ mi_cmd_exec_run (const char *command, char **argv, int argc)
   if (oind != argc)
     error (_("Invalid argument: %s"), argv[oind]);
 
+  const char *run_cmd = start_p ? "start" : "run";
   if (current_context->all)
     {
       scoped_restore_current_pspace_and_thread restore_pspace_thread;
 
-      iterate_over_inferiors (run_one_inferior, &start_p);
+      run_inferiors (run_cmd);
     }
   else
     {
-      const char *run_cmd = start_p ? "start" : "run";
       struct target_ops *run_target = find_run_target ();
       int async_p = mi_async && run_target->can_async_p ();
 
-- 
2.17.2

[PATCH 2/4] Don't use iterate_over_inferiors in top.c

[Tom Tromey]

This changes top.c to use iterators rather than
iterate_over_inferiors.

gdb/ChangeLog
2019-09-19  Tom Tromey  <tom@tromey.com>

	* top.c (struct qt_args): Remove.
	(kill_or_detach): Change type.
	(quit_force): Update.
	(print_inferior_quit_actions): Rename from
	print_inferior_quit_action.  Change type.
	(quit_confirm): Update.
---
 gdb/ChangeLog |  9 ++++++
 gdb/top.c     | 90 +++++++++++++++++++++++----------------------------
 2 files changed, 49 insertions(+), 50 deletions(-)

diff --git a/gdb/top.c b/gdb/top.c
index 49e6daae949..276780f7219 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1548,62 +1548,55 @@ set_prompt (const char *s)
 }
 \f
 
-struct qt_args
-{
-  int from_tty;
-};
-
-/* Callback for iterate_over_inferiors.  Kills or detaches the given
-   inferior, depending on how we originally gained control of it.  */
+/* Kill or detach each inferior, depending on how we originally gained
+   control of it.  */
 
-static int
-kill_or_detach (struct inferior *inf, void *args)
+static void
+kill_or_detach (int from_tty)
 {
-  struct qt_args *qt = (struct qt_args *) args;
-
-  if (inf->pid == 0)
-    return 0;
-
-  thread_info *thread = any_thread_of_inferior (inf);
-  if (thread != NULL)
+  for (inferior *inf : all_inferiors_safe ())
     {
-      switch_to_thread (thread);
+      if (inf->pid == 0)
+	continue;
 
-      /* Leave core files alone.  */
-      if (target_has_execution)
+      thread_info *thread = any_thread_of_inferior (inf);
+      if (thread != NULL)
 	{
-	  if (inf->attach_flag)
-	    target_detach (inf, qt->from_tty);
-	  else
-	    target_kill ();
+	  switch_to_thread (thread);
+
+	  /* Leave core files alone.  */
+	  if (target_has_execution)
+	    {
+	      if (inf->attach_flag)
+		target_detach (inf, from_tty);
+	      else
+		target_kill ();
+	    }
 	}
     }
-
-  return 0;
 }
 
-/* Callback for iterate_over_inferiors.  Prints info about what GDB
-   will do to each inferior on a "quit".  ARG points to a struct
-   ui_out where output is to be collected.  */
+/* Print info about what GDB will do to each inferior on a "quit".  */
 
-static int
-print_inferior_quit_action (struct inferior *inf, void *arg)
+static void
+print_inferior_quit_actions (struct ui_file *stb)
 {
-  struct ui_file *stb = (struct ui_file *) arg;
-
-  if (inf->pid == 0)
-    return 0;
-
-  if (inf->attach_flag)
-    fprintf_filtered (stb,
-		      _("\tInferior %d [%s] will be detached.\n"), inf->num,
-		      target_pid_to_str (ptid_t (inf->pid)).c_str ());
-  else
-    fprintf_filtered (stb,
-		      _("\tInferior %d [%s] will be killed.\n"), inf->num,
-		      target_pid_to_str (ptid_t (inf->pid)).c_str ());
-
-  return 0;
+  for (inferior *inf : all_inferiors_safe ())
+    {
+      if (inf->pid == 0)
+	continue;
+
+      if (inf->attach_flag)
+	fprintf_filtered (stb,
+			  _("\tInferior %d [%s] will be detached.\n"),
+			  inf->num,
+			  target_pid_to_str (ptid_t (inf->pid)).c_str ());
+      else
+	fprintf_filtered (stb,
+			  _("\tInferior %d [%s] will be killed.\n"),
+			  inf->num,
+			  target_pid_to_str (ptid_t (inf->pid)).c_str ());
+    }
 }
 
 /* If necessary, make the user confirm that we should quit.  Return
@@ -1620,7 +1613,7 @@ quit_confirm (void)
   string_file stb;
 
   stb.puts (_("A debugging session is active.\n\n"));
-  iterate_over_inferiors (print_inferior_quit_action, &stb);
+  print_inferior_quit_actions (&stb);
   stb.puts (_("\nQuit anyway? "));
 
   return query ("%s", stb.c_str ());
@@ -1653,7 +1646,6 @@ void
 quit_force (int *exit_arg, int from_tty)
 {
   int exit_code = 0;
-  struct qt_args qt;
 
   undo_terminal_modifications_before_exit ();
 
@@ -1664,15 +1656,13 @@ quit_force (int *exit_arg, int from_tty)
   else if (return_child_result)
     exit_code = return_child_result_value;
 
-  qt.from_tty = from_tty;
-
   /* We want to handle any quit errors and exit regardless.  */
 
   /* Get out of tfind mode, and kill or detach all inferiors.  */
   try
     {
       disconnect_tracing ();
-      iterate_over_inferiors (kill_or_detach, &qt);
+      kill_or_detach (from_tty);
     }
   catch (const gdb_exception &ex)
     {
-- 
2.17.2

Re: [PATCH 0/4] Remove some uses of iterate_over_inferiors

[Simon Marchi]

On 2019-09-19 5:28 p.m., Tom Tromey wrote:
> This series removes some uses of iterate_over_inferiors in favor of
> foreach-style iteration.  I find the latter simpler both to debug and
> to read.
> 
> There are more instances of iterate_over_inferiors, and other
> iterate_over_* functions -- this series is just a starter to see
> whether others agree, so let me know what you think.
> 
> Tested on x86-64 Fedora by the buildbot.
> 
> Tom
> 
> 

Hi Tom,

This all looks good to me, I agree with the direction.

I was just wondering, why did you choose to use all_inferiors_safe in all
these cases, rather than all_inferiors?  At first sight, they don't seem
to be cases where an inferior can be deleted.  Is it maybe that it's
simpler to just use the "safe" version and there's no disadvantage to it?

Simon

Re: [PATCH 0/4] Remove some uses of iterate_over_inferiors

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> I was just wondering, why did you choose to use
Simon> all_inferiors_safe in all these cases, rather than all_inferiors?
Simon> At first sight, they don't seem to be cases where an inferior can
Simon> be deleted.  Is it maybe that it's simpler to just use the "safe"
Simon> version and there's no disadvantage to it?

It was just more obviously correct, because that's what
iterate_over_inferiors uses.

Tom