diff mbox

Introduce utility function find_inferior_ptid

Message ID 1418412193-6259-1-git-send-email-simon.marchi@ericsson.com
State Superseded
Headers

Commit Message

Simon Marchi Dec. 12, 2014, 7:23 p.m. UTC 
  From: Simon Marchi <simon.marchi@ericsson.com>

This patch introduces find_inferior_ptid to replace the common idiom

  find_inferior_pid (ptid_get_pid (...));

It replaces all the instances of that idiom that I found with the new
function.

The change was mostly mechanical and built-tested.

gdb/ChangeLog:

	* inferior.c (find_inferior_ptid): New function.
	* inferior.h (find_inferior_ptid): New declaration.
	* ada-tasks.c (ada_get_task_number): Use find_inferior_ptid.
	* corelow.c (core_pid_to_str): Same.
	* darwin-nat.c (darwin_resume): Same.
	* infrun.c (fetch_inferior_event): Same.
	(get_inferior_stop_soon): Same.
	(handle_inferior_event): Same.
	(handle_signal_stop): Same.
	* linux-nat.c (resume_lwp): Same.
	(stop_wait_callback): Same.
	* mi/mi-interp.c (mi_new_thread): Same.
	(mi_thread_exit): Same.
	* proc-service.c (ps_pglobal_lookup): Same.
	* record-btrace.c (record_btrace_step_thread): Same.
	* remote-sim.c (gdbsim_close_inferior): Same.
	(gdbsim_resume): Same.
	(gdbsim_stop): Same.
	* sol2-tdep.c (sol2_core_pid_to_str): Same.
	* target.c (memory_xfer_partial_1): Same.
	(default_thread_address_space): Same.
	* thread.c (thread_change_ptid): Same.
	(switch_to_thread): Same.
	(do_restore_current_thread_cleanup): Same.
---
 gdb/ada-tasks.c     |  3 +--
 gdb/corelow.c       |  2 +-
 gdb/darwin-nat.c    |  2 +-
 gdb/inferior.c      |  5 +++++
 gdb/inferior.h      |  3 +++
 gdb/infrun.c        | 10 +++++-----
 gdb/linux-nat.c     |  4 ++--
 gdb/mi/mi-interp.c  |  4 ++--
 gdb/proc-service.c  |  2 +-
 gdb/record-btrace.c |  4 ++--
 gdb/remote-sim.c    |  6 +++---
 gdb/sol2-tdep.c     |  2 +-
 gdb/target.c        |  4 ++--
 gdb/thread.c        |  6 +++---
 14 files changed, 32 insertions(+), 25 deletions(-)

Comments

Joel Brobecker Dec. 13, 2014, 2:40 p.m. UTC | #1 
> This patch introduces find_inferior_ptid to replace the common idiom
> 
>   find_inferior_pid (ptid_get_pid (...));
> 
> It replaces all the instances of that idiom that I found with the new
> function.
> 
> The change was mostly mechanical and built-tested.
> 
> gdb/ChangeLog:
> 
> 	* inferior.c (find_inferior_ptid): New function.
> 	* inferior.h (find_inferior_ptid): New declaration.
> 	* ada-tasks.c (ada_get_task_number): Use find_inferior_ptid.
> 	* corelow.c (core_pid_to_str): Same.
> 	* darwin-nat.c (darwin_resume): Same.
> 	* infrun.c (fetch_inferior_event): Same.
> 	(get_inferior_stop_soon): Same.
> 	(handle_inferior_event): Same.
> 	(handle_signal_stop): Same.
> 	* linux-nat.c (resume_lwp): Same.
> 	(stop_wait_callback): Same.
> 	* mi/mi-interp.c (mi_new_thread): Same.
> 	(mi_thread_exit): Same.
> 	* proc-service.c (ps_pglobal_lookup): Same.
> 	* record-btrace.c (record_btrace_step_thread): Same.
> 	* remote-sim.c (gdbsim_close_inferior): Same.
> 	(gdbsim_resume): Same.
> 	(gdbsim_stop): Same.
> 	* sol2-tdep.c (sol2_core_pid_to_str): Same.
> 	* target.c (memory_xfer_partial_1): Same.
> 	(default_thread_address_space): Same.
> 	* thread.c (thread_change_ptid): Same.
> 	(switch_to_thread): Same.
> 	(do_restore_current_thread_cleanup): Same.

Overall, this looks reasonable. A couple of comments below.
Also, while at it, I would prefer if you tested the change.
With the number of mechanical changes, it's really easy to
let one bad change slip by, no matter how careful we try
to be; testing does not cost much so let's add that as well.

>  
> +struct inferior *
> +find_inferior_ptid (ptid_t ptid) {
> +  return find_inferior_ptid (ptid);
> +}

This function needs an introductory commend ("/* See inferior.h */").
This is something we do systematically now, even if the function's
documentation is present elsewhere. The one-line comment allows us
to know that there is a command, in incidentally where it is.

Also, the opening curly brace needs to be on the next line. Therefore:

        /* See inferior.h.  */

        struct inferior *
        find_inferior_ptid (ptid_t ptid)
        {
          return find_inferior_ptid (ptid);
        }

> +/* Search function to lookup an inferior whose pid is equal to 'ptid.pid'. */
> +extern struct inferior * find_inferior_ptid (ptid_t ptid);

No space after the '*'.

Pre-approved with those changes and after testing confirmed.

Thank you,
diff mbox

Patch

diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index 2d5a19d..17d0338 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -292,7 +292,7 @@  int
 ada_get_task_number (ptid_t ptid)
 {
   int i;
-  struct inferior *inf = find_inferior_pid (ptid_get_pid (ptid));
+  struct inferior *inf = find_inferior_ptid (ptid);
   struct ada_tasks_inferior_data *data;
 
   gdb_assert (inf != NULL);
@@ -1449,4 +1449,3 @@  _initialize_tasks (void)
 Without argument, this command simply prints the current task ID"),
            &cmdlist);
 }
-
diff --git a/gdb/corelow.c b/gdb/corelow.c
index b91ad22..154b2c4 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -954,7 +954,7 @@  core_pid_to_str (struct target_ops *ops, ptid_t ptid)
 
   /* Otherwise, this isn't a "threaded" core -- use the PID field, but
      only if it isn't a fake PID.  */
-  inf = find_inferior_pid (ptid_get_pid (ptid));
+  inf = find_inferior_ptid (ptid);
   if (inf != NULL && !inf->fake_pid_p)
     return normal_pid_to_str (ptid);
 
diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index 511f370..9ddc8d4 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -859,7 +859,7 @@  darwin_resume (ptid_t ptid, int step, enum gdb_signal signal)
     }
   else
     {
-      struct inferior *inf = find_inferior_pid (ptid_get_pid (ptid));
+      struct inferior *inf = find_inferior_ptid (ptid));
       long tid = ptid_get_tid (ptid);
 
       /* Stop the inferior (should be useless).  */
diff --git a/gdb/inferior.c b/gdb/inferior.c
index 44f4560..eb4e45d 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -367,6 +367,11 @@  find_inferior_pid (int pid)
   return NULL;
 }
 
+struct inferior *
+find_inferior_ptid (ptid_t ptid) {
+  return find_inferior_ptid (ptid);
+}
+
 /* See inferior.h.  */
 
 struct inferior *
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 0129549..0856c64 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -465,6 +465,9 @@  extern int valid_gdb_inferior_id (int num);
 /* Search function to lookup an inferior by target 'pid'.  */
 extern struct inferior *find_inferior_pid (int pid);
 
+/* Search function to lookup an inferior whose pid is equal to 'ptid.pid'. */
+extern struct inferior * find_inferior_ptid (ptid_t ptid);
+
 /* Search function to lookup an inferior by GDB 'num'.  */
 extern struct inferior *find_inferior_id (int num);
 
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 74f9e12..413b052 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3262,7 +3262,7 @@  fetch_inferior_event (void *client_data)
 
   if (!ecs->wait_some_more)
     {
-      struct inferior *inf = find_inferior_pid (ptid_get_pid (ecs->ptid));
+      struct inferior *inf = find_inferior_ptid (ecs->ptid);
 
       delete_just_stopped_threads_infrun_breakpoints ();
 
@@ -3581,7 +3581,7 @@  fill_in_stop_func (struct gdbarch *gdbarch,
 static enum stop_kind
 get_inferior_stop_soon (ptid_t ptid)
 {
-  struct inferior *inf = find_inferior_pid (ptid_get_pid (ptid));
+  struct inferior *inf = find_inferior_ptid (ptid);
 
   gdb_assert (inf != NULL);
   return inf->control.stop_soon;
@@ -3815,7 +3815,7 @@  handle_inferior_event (struct execution_control_state *ecs)
 	}
 
       inferior_ptid = ecs->ptid;
-      set_current_inferior (find_inferior_pid (ptid_get_pid (ecs->ptid)));
+      set_current_inferior (find_inferior_ptid (ecs->ptid));
       set_current_program_space (current_inferior ()->pspace);
       handle_vfork_child_exec_or_exit (0);
       target_terminal_ours ();	/* Must do this before mourn anyway.  */
@@ -3899,7 +3899,7 @@  Cannot fill $_exitsignal with the correct signal number.\n"));
 	if (displaced && ptid_equal (displaced->step_ptid, ecs->ptid))
 	  {
 	    struct inferior *parent_inf
-	      = find_inferior_pid (ptid_get_pid (ecs->ptid));
+	      = find_inferior_ptid (ecs->ptid);
 	    struct regcache *child_regcache;
 	    CORE_ADDR parent_pc;
 
@@ -4495,7 +4495,7 @@  handle_signal_stop (struct execution_control_state *ecs)
   if (random_signal)
     {
       /* Signal not for debugging purposes.  */
-      struct inferior *inf = find_inferior_pid (ptid_get_pid (ecs->ptid));
+      struct inferior *inf = find_inferior_ptid (ecs->ptid);
       enum gdb_signal stop_signal = ecs->event_thread->suspend.stop_signal;
 
       if (debug_infrun)
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 21797c1..29133f9 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1435,7 +1435,7 @@  resume_lwp (struct lwp_info *lp, int step, enum gdb_signal signo)
 {
   if (lp->stopped)
     {
-      struct inferior *inf = find_inferior_pid (ptid_get_pid (lp->ptid));
+      struct inferior *inf = find_inferior_ptid (lp->ptid);
 
       if (inf->vfork_child != NULL)
 	{
@@ -2388,7 +2388,7 @@  linux_nat_set_status_is_event (struct target_ops *t,
 static int
 stop_wait_callback (struct lwp_info *lp, void *data)
 {
-  struct inferior *inf = find_inferior_pid (ptid_get_pid (lp->ptid));
+  struct inferior *inf = find_inferior_ptid (lp->ptid);
 
   /* If this is a vfork parent, bail out, it is not going to report
      any SIGSTOP until the vfork is done with.  */
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 60f0666..09ff7bf 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -371,7 +371,7 @@  static void
 mi_new_thread (struct thread_info *t)
 {
   struct mi_interp *mi = top_level_interpreter_data ();
-  struct inferior *inf = find_inferior_pid (ptid_get_pid (t->ptid));
+  struct inferior *inf = find_inferior_ptid (t->ptid);
 
   gdb_assert (inf);
 
@@ -391,7 +391,7 @@  mi_thread_exit (struct thread_info *t, int silent)
   if (silent)
     return;
 
-  inf = find_inferior_pid (ptid_get_pid (t->ptid));
+  inf = find_inferior_ptid (t->ptid);
 
   mi = top_level_interpreter_data ();
   old_chain = make_cleanup_restore_target_terminal ();
diff --git a/gdb/proc-service.c b/gdb/proc-service.c
index 5819489..1dc98d8 100644
--- a/gdb/proc-service.c
+++ b/gdb/proc-service.c
@@ -114,7 +114,7 @@  ps_pglobal_lookup (gdb_ps_prochandle_t ph, const char *obj,
 {
   struct bound_minimal_symbol ms;
   struct cleanup *old_chain = save_current_program_space ();
-  struct inferior *inf = find_inferior_pid (ptid_get_pid (ph->ptid));
+  struct inferior *inf = find_inferior_ptid (ph->ptid);
   ps_err_e result;
 
   set_current_program_space (inf->pspace);
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 65d28e3..8b98a91 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -1662,7 +1662,7 @@  record_btrace_step_thread (struct thread_info *tp)
       if (replay == NULL)
 	return btrace_step_no_history ();
 
-      inf = find_inferior_pid (ptid_get_pid (tp->ptid));
+      inf = find_inferior_ptid (tp->ptid);
       aspace = inf->aspace;
 
       /* Determine the end of the instruction trace.  */
@@ -1699,7 +1699,7 @@  record_btrace_step_thread (struct thread_info *tp)
       if (replay == NULL)
 	replay = record_btrace_start_replaying (tp);
 
-      inf = find_inferior_pid (ptid_get_pid (tp->ptid));
+      inf = find_inferior_ptid (tp->ptid);
       aspace = inf->aspace;
 
       for (;;)
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index 8901548..3235b19 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -774,7 +774,7 @@  gdbsim_close_inferior (struct inferior *inf, void *arg)
 	 Thus we need to verify the existence of an inferior using the
 	 pid in question before setting inferior_ptid via
 	 switch_to_thread() or mourning the inferior.  */
-      if (find_inferior_pid (ptid_get_pid (ptid)) != NULL)
+      if (find_inferior_ptid (ptid)) != NULL)
 	{
 	  switch_to_thread (ptid);
 	  generic_mourn_inferior ();
@@ -881,7 +881,7 @@  gdbsim_resume (struct target_ops *ops,
      either have multiple inferiors to resume or an error condition.  */
 
   if (sim_data)
-    gdbsim_resume_inferior (find_inferior_pid (ptid_get_pid (ptid)), &rd);
+    gdbsim_resume_inferior (find_inferior_ptid (ptid)), &rd);
   else if (ptid_equal (ptid, minus_one_ptid))
     iterate_over_inferiors (gdbsim_resume_inferior, &rd);
   else
@@ -928,7 +928,7 @@  gdbsim_stop (struct target_ops *self, ptid_t ptid)
     }
   else
     {
-      struct inferior *inf = find_inferior_pid (ptid_get_pid (ptid));
+      struct inferior *inf = find_inferior_ptid (ptid));
 
       if (inf == NULL)
 	error (_("Can't stop pid %d.  No inferior found."),
diff --git a/gdb/sol2-tdep.c b/gdb/sol2-tdep.c
index bf7d6a1..88a5aea 100644
--- a/gdb/sol2-tdep.c
+++ b/gdb/sol2-tdep.c
@@ -60,7 +60,7 @@  sol2_core_pid_to_str (struct gdbarch *gdbarch, ptid_t ptid)
   /* GDB didn't use to put a NT_PSTATUS note in Solaris cores.  If
      that's missing, then we're dealing with a fake PID corelow.c made
      up.  */
-  inf = find_inferior_pid (ptid_get_pid (ptid));
+  inf = find_inferior_ptid (ptid));
   if (inf == NULL || inf->fake_pid_p)
     {
       xsnprintf (buf, sizeof buf, "<core>");
diff --git a/gdb/target.c b/gdb/target.c
index 7161e62..38c6c16 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1173,7 +1173,7 @@  memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
     return TARGET_XFER_E_IO;
 
   if (!ptid_equal (inferior_ptid, null_ptid))
-    inf = find_inferior_pid (ptid_get_pid (inferior_ptid));
+    inf = find_inferior_ptid (inferior_ptid);
   else
     inf = NULL;
 
@@ -2649,7 +2649,7 @@  default_thread_address_space (struct target_ops *self, ptid_t ptid)
   struct inferior *inf;
 
   /* Fall-back to the "main" address space of the inferior.  */
-  inf = find_inferior_pid (ptid_get_pid (ptid));
+  inf = find_inferior_ptid (ptid);
 
   if (inf == NULL || inf->aspace == NULL)
     internal_error (__FILE__, __LINE__,
diff --git a/gdb/thread.c b/gdb/thread.c
index a3040a7..782fc70 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -747,7 +747,7 @@  thread_change_ptid (ptid_t old_ptid, ptid_t new_ptid)
   /* It can happen that what we knew as the target inferior id
      changes.  E.g, target remote may only discover the remote process
      pid after adding the inferior to GDB's list.  */
-  inf = find_inferior_pid (ptid_get_pid (old_ptid));
+  inf = find_inferior_ptid (old_ptid);
   inf->pid = ptid_get_pid (new_ptid);
 
   tp = find_thread_ptid (old_ptid);
@@ -1179,7 +1179,7 @@  switch_to_thread (ptid_t ptid)
     {
       struct inferior *inf;
 
-      inf = find_inferior_pid (ptid_get_pid (ptid));
+      inf = find_inferior_ptid (ptid);
       gdb_assert (inf != NULL);
       set_current_program_space (inf->pspace);
       set_current_inferior (inf);
@@ -1290,7 +1290,7 @@  do_restore_current_thread_cleanup (void *arg)
      then don't revert back to it, but instead simply drop back to no
      thread selected.  */
   if (tp
-      && find_inferior_pid (ptid_get_pid (tp->ptid)) != NULL)
+      && find_inferior_ptid (tp->ptid) != NULL)
     restore_current_thread (old->inferior_ptid);
   else
     {