Patchwork [3/3] Make linux_nat_detach/thread_db_detach use the inferior parameter

login
register
mail settings
Submitter Simon Marchi
Date Dec. 31, 2017, 5:50 a.m.
Message ID <1514699454-18587-3-git-send-email-simon.marchi@ericsson.com>
Download mbox | patch
Permalink /patch/25160/
State New
Headers show

Comments

Simon Marchi - Dec. 31, 2017, 5:50 a.m.
This patch makes these two functions actually use the inferior parameter
added by the previous patch, instead of reading inferior_ptid.  I chose
these two, because they are the one actually used when I detach on my
GNU/Linux system, so they were easy to test.

I took the opportunity to pass the inferior being detached to
inf_ptrace_detach_success, so it could use it too.  From there, it made
sense to add an overload of detach_inferior that takes the inferior
directly rather than the pid, to avoid having to pass inf->pid only for
the callee to look up the inferior structure by pid.

gdb/ChangeLog:

	* inf-ptrace.c (inf_ptrace_detach): Adjust call to
	inf_ptrace_detach_success.
	(inf_ptrace_detach_success): Add inferior parameter, use it
	instead of inferior_ptid, pass it to detach_inferior.
	* inf-ptrace.h (inf_ptrace_detach_success): Add inferior
	parameter.
	* inferior.c (detach_inferior): Add overload that takes an
	inferior object.
	* inferior.h (detach_inferior): Likewise.
	* linux-nat.c (linux_nat_detach): Use the inf parameter, don't
	use inferior_ptid, adjust call to inf_ptrace_detach_success.
	* linux-thread-db.c (thread_db_detach): Use inf parameter.
---
 gdb/inf-ptrace.c      |  8 +++-----
 gdb/inf-ptrace.h      |  2 +-
 gdb/inferior.c        | 15 +++++++++++++--
 gdb/inferior.h        |  3 +++
 gdb/linux-nat.c       |  8 +++-----
 gdb/linux-thread-db.c |  2 +-
 6 files changed, 24 insertions(+), 14 deletions(-)
Pedro Alves - Jan. 18, 2018, 5:06 p.m.
On 12/31/2017 05:50 AM, Simon Marchi wrote:
> This patch makes these two functions actually use the inferior parameter
> added by the previous patch, instead of reading inferior_ptid.  I chose
> these two, because they are the one actually used when I detach on my
> GNU/Linux system, so they were easy to test.
> 
> I took the opportunity to pass the inferior being detached to
> inf_ptrace_detach_success, so it could use it too.  From there, it made
> sense to add an overload of detach_inferior that takes the inferior
> directly rather than the pid, to avoid having to pass inf->pid only for
> the callee to look up the inferior structure by pid.

Looks fine to me.

(FWIW, the 'detach_inferior(int)' overload disappears in
my multi-target branch).

Thanks,
Pedro Alves
Simon Marchi - Jan. 19, 2018, 3:26 a.m.
On 2018-01-18 12:06, Pedro Alves wrote:
> On 12/31/2017 05:50 AM, Simon Marchi wrote:
>> This patch makes these two functions actually use the inferior 
>> parameter
>> added by the previous patch, instead of reading inferior_ptid.  I 
>> chose
>> these two, because they are the one actually used when I detach on my
>> GNU/Linux system, so they were easy to test.
>> 
>> I took the opportunity to pass the inferior being detached to
>> inf_ptrace_detach_success, so it could use it too.  From there, it 
>> made
>> sense to add an overload of detach_inferior that takes the inferior
>> directly rather than the pid, to avoid having to pass inf->pid only 
>> for
>> the callee to look up the inferior structure by pid.
> 
> Looks fine to me.
> 
> (FWIW, the 'detach_inferior(int)' overload disappears in
> my multi-target branch).

Ah, good to know it goes in the same direction.

I'm sending the series through the buildbot again (see if that new 
assert is hit somewhere), and will send a v2 tomorrow if all goes well.

Thanks for reviewing.

Simon

Patch

diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index 766a660..c6318d4 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -263,18 +263,16 @@  inf_ptrace_detach (struct target_ops *ops, inferior *inf, int from_tty)
   error (_("This system does not support detaching from a process"));
 #endif
 
-  inf_ptrace_detach_success (ops);
+  inf_ptrace_detach_success (ops, inf);
 }
 
 /* See inf-ptrace.h.  */
 
 void
-inf_ptrace_detach_success (struct target_ops *ops)
+inf_ptrace_detach_success (struct target_ops *ops, inferior *inf)
 {
-  pid_t pid = ptid_get_pid (inferior_ptid);
-
   inferior_ptid = null_ptid;
-  detach_inferior (pid);
+  detach_inferior (inf);
 
   inf_child_maybe_unpush_target (ops);
 }
diff --git a/gdb/inf-ptrace.h b/gdb/inf-ptrace.h
index 3f27efe..61dc091 100644
--- a/gdb/inf-ptrace.h
+++ b/gdb/inf-ptrace.h
@@ -40,6 +40,6 @@  extern pid_t get_ptrace_pid (ptid_t);
 
 
 /* Cleanup the inferior after a successful ptrace detach.  */
-extern void inf_ptrace_detach_success (struct target_ops *ops);
+extern void inf_ptrace_detach_success (struct target_ops *ops, inferior *inf);
 
 #endif
diff --git a/gdb/inferior.c b/gdb/inferior.c
index 9b3043d..2db358e 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -253,10 +253,13 @@  exit_inferior_num_silent (int num)
   exit_inferior_1 (inf, 1);
 }
 
+/* See inferior.h.  */
+
 void
-detach_inferior (int pid)
+detach_inferior (inferior *inf)
 {
-  struct inferior *inf = find_inferior_pid (pid);
+  /* Save the pid, since exit_inferior_1 will reset it.  */
+  int pid = inf->pid;
 
   exit_inferior_1 (inf, 0);
 
@@ -264,6 +267,14 @@  detach_inferior (int pid)
     printf_unfiltered (_("[Inferior %d detached]\n"), pid);
 }
 
+/* See inferior.h.  */
+
+void
+detach_inferior (int pid)
+{
+  detach_inferior (find_inferior_pid (pid));
+}
+
 void
 inferior_appeared (struct inferior *inf, int pid)
 {
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 0705dd9..dd4addf 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -458,6 +458,9 @@  extern struct inferior *add_inferior_silent (int pid);
 extern void delete_inferior (struct inferior *todel);
 
 /* Delete an existing inferior list entry, due to inferior detaching.  */
+extern void detach_inferior (inferior *inf);
+
+/* Same as the above, but with the inferior specified by PID.  */
 extern void detach_inferior (int pid);
 
 extern void exit_inferior (int pid);
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 9a54397..a8793d8 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1509,10 +1509,8 @@  detach_callback (struct lwp_info *lp, void *data)
 static void
 linux_nat_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
-  int pid;
   struct lwp_info *main_lwp;
-
-  pid = ptid_get_pid (inferior_ptid);
+  int pid = inf->pid;
 
   /* Don't unregister from the event loop, as there may be other
      inferiors running. */
@@ -1527,7 +1525,7 @@  linux_nat_detach (struct target_ops *ops, inferior *inf, int from_tty)
   iterate_over_lwps (pid_to_ptid (pid), detach_callback, NULL);
 
   /* Only the initial process should be left right now.  */
-  gdb_assert (num_lwps (ptid_get_pid (inferior_ptid)) == 1);
+  gdb_assert (num_lwps (pid) == 1);
 
   main_lwp = find_lwp_pid (pid_to_ptid (pid));
 
@@ -1548,7 +1546,7 @@  linux_nat_detach (struct target_ops *ops, inferior *inf, int from_tty)
 
       detach_one_lwp (main_lwp, &signo);
 
-      inf_ptrace_detach_success (ops);
+      inf_ptrace_detach_success (ops, inf);
     }
 }
 
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index ddb010e..eb5e49b 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -1094,7 +1094,7 @@  thread_db_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
   struct target_ops *target_beneath = find_target_beneath (ops);
 
-  delete_thread_db_info (ptid_get_pid (inferior_ptid));
+  delete_thread_db_info (inf->pid);
 
   target_beneath->to_detach (target_beneath, inf, from_tty);