[review] infrun: extract out a code piece into the 'handle_inferior_exit' func...

Message ID gerrit.1572863015000.Id8c7d485805cf39623c3e06b4a565515b7e9625c@gnutoolchain-gerrit.osci.io
State New
Headers show
Series
  • [review] infrun: extract out a code piece into the 'handle_inferior_exit' func...
Related show

Commit Message

Sourceware to Gerrit sync (Code Review) Nov. 4, 2019, 10:23 a.m.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/508
......................................................................

infrun: extract out a code piece into the 'handle_inferior_exit' function

This is a refactoring.  The extracted function is placed deliberately
before 'stop_all_threads' because the the function will be re-used
there in a subsequent patch for handling a TARGET_WAITKIND_EXITED or
TARGET_WAITKIND_SIGNALLED status kind received from a thread that GDB
attempted to stop.

gdb/ChangeLog:
2019-11-04  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* infrun.c (handle_inferior_event): Extract out a piece of code
	into...
	(handle_inferior_exit): ...this new function.

Change-Id: Id8c7d485805cf39623c3e06b4a565515b7e9625c
---
M gdb/infrun.c
1 file changed, 69 insertions(+), 56 deletions(-)




-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Id8c7d485805cf39623c3e06b4a565515b7e9625c
Gerrit-Change-Number: 508
Gerrit-PatchSet: 1
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-MessageType: newchange

Comments

Sourceware to Gerrit sync (Code Review) Dec. 6, 2019, 3:45 p.m. | #1
Pedro Alves has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/508
......................................................................


Patch Set 1: Code-Review+2

(2 comments)

LGTM.  See nits below.

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +1,19 @@ 
| +Parent:     f5da53f6 (infrun: extract out a code piece into 'mark_non_executing_threads' function)
| +Author:     Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
| +AuthorDate: 2019-11-04 11:21:38 +0100
| +Commit:     Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
| +CommitDate: 2019-11-04 11:21:38 +0100
| +
| +infrun: extract out a code piece into the 'handle_inferior_exit' function
| +
| +This is a refactoring.  The extracted function is placed deliberately
| +before 'stop_all_threads' because the the function will be re-used

PS1, Line 10:

typo: "the the"

| +there in a subsequent patch for handling a TARGET_WAITKIND_EXITED or
| +TARGET_WAITKIND_SIGNALLED status kind received from a thread that GDB
| +attempted to stop.
| +
| +gdb/ChangeLog:
| +2019-11-04  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
| +
| +	* infrun.c (handle_inferior_event): Extract out a piece of code
| +	into...
| --- gdb/infrun.c
| +++ gdb/infrun.c
| @@ -4316,4 +4316,19 @@ mark_non_executing_threads (ptid_t event_ptid,
|    /* Likewise the resumed flag.  */
|    set_resumed (mark_ptid, 0);
|  }
|  
| +/* Handle a TARGET_WAITKIND_EXITED or TARGET_WAITKIND_SIGNALLED
| +   event.  */
| +
| +static void
| +handle_inferior_exit (ptid_t event_ptid,
| +		      struct target_waitstatus ws)

PS1, Line 4325:

This all fits in a single line.  Even more so if you remove the
redundant "struct".

(probably the same applies in the previous patch.)

| +{
| +  gdb_assert (ws.kind == TARGET_WAITKIND_EXITED
| +	      || ws.kind == TARGET_WAITKIND_SIGNALLED);
| +
| +  inferior_ptid = event_ptid;
| +  set_current_inferior (find_inferior_ptid (event_ptid));
| +  set_current_program_space (current_inferior ()->pspace);
| +  handle_vfork_child_exec_or_exit (0);
| +  target_terminal::ours ();	/* Must do this before mourn anyway.  */

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Id8c7d485805cf39623c3e06b4a565515b7e9625c
Gerrit-Change-Number: 508
Gerrit-PatchSet: 1
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Fri, 06 Dec 2019 15:45:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Patch

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 57405d7..f628fd1 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4317,6 +4317,74 @@ 
   set_resumed (mark_ptid, 0);
 }
 
+/* Handle a TARGET_WAITKIND_EXITED or TARGET_WAITKIND_SIGNALLED
+   event.  */
+
+static void
+handle_inferior_exit (ptid_t event_ptid,
+		      struct target_waitstatus ws)
+{
+  gdb_assert (ws.kind == TARGET_WAITKIND_EXITED
+	      || ws.kind == TARGET_WAITKIND_SIGNALLED);
+
+  inferior_ptid = event_ptid;
+  set_current_inferior (find_inferior_ptid (event_ptid));
+  set_current_program_space (current_inferior ()->pspace);
+  handle_vfork_child_exec_or_exit (0);
+  target_terminal::ours ();	/* Must do this before mourn anyway.  */
+
+  /* Clearing any previous state of convenience variables.  */
+  clear_exit_convenience_vars ();
+
+  if (ws.kind == TARGET_WAITKIND_EXITED)
+    {
+      /* Record the exit code in the convenience variable $_exitcode, so
+	 that the user can inspect this again later.  */
+      set_internalvar_integer (lookup_internalvar ("_exitcode"),
+			       (LONGEST) ws.value.integer);
+
+      /* Also record this in the inferior itself.  */
+      current_inferior ()->has_exit_code = 1;
+      current_inferior ()->exit_code = (LONGEST) ws.value.integer;
+
+      /* Support the --return-child-result option.  */
+      return_child_result_value = ws.value.integer;
+
+      gdb::observers::exited.notify (ws.value.integer);
+    }
+  else
+    {
+      struct gdbarch *gdbarch = current_inferior ()->gdbarch;
+
+      if (gdbarch_gdb_signal_to_target_p (gdbarch))
+	{
+	  /* Set the value of the internal variable $_exitsignal,
+	     which holds the signal uncaught by the inferior.  */
+	  set_internalvar_integer (lookup_internalvar ("_exitsignal"),
+				   gdbarch_gdb_signal_to_target (gdbarch,
+								 ws.value.sig));
+	}
+      else
+	{
+	  /* We don't have access to the target's method used for
+	     converting between signal numbers (GDB's internal
+	     representation <-> target's representation).
+	     Therefore, we cannot do a good job at displaying this
+	     information to the user.  It's better to just warn
+	     her about it (if infrun debugging is enabled), and
+	     give up.  */
+	  if (debug_infrun)
+	    fprintf_filtered (gdb_stdlog, _("\
+Cannot fill $_exitsignal with the correct signal number.\n"));
+	}
+
+      gdb::observers::signal_exited.notify (ws.value.sig);
+    }
+
+  gdb_flush (gdb_stdout);
+  target_mourn_inferior (inferior_ptid);
+}
+
 /* See infrun.h.  */
 
 void
@@ -4842,62 +4910,7 @@ 
 
     case TARGET_WAITKIND_EXITED:
     case TARGET_WAITKIND_SIGNALLED:
-      inferior_ptid = 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.  */
-
-      /* Clearing any previous state of convenience variables.  */
-      clear_exit_convenience_vars ();
-
-      if (ecs->ws.kind == TARGET_WAITKIND_EXITED)
-	{
-	  /* Record the exit code in the convenience variable $_exitcode, so
-	     that the user can inspect this again later.  */
-	  set_internalvar_integer (lookup_internalvar ("_exitcode"),
-				   (LONGEST) ecs->ws.value.integer);
-
-	  /* Also record this in the inferior itself.  */
-	  current_inferior ()->has_exit_code = 1;
-	  current_inferior ()->exit_code = (LONGEST) ecs->ws.value.integer;
-
-	  /* Support the --return-child-result option.  */
-	  return_child_result_value = ecs->ws.value.integer;
-
-	  gdb::observers::exited.notify (ecs->ws.value.integer);
-	}
-      else
-	{
-	  struct gdbarch *gdbarch = current_inferior ()->gdbarch;
-
-	  if (gdbarch_gdb_signal_to_target_p (gdbarch))
-	    {
-	      /* Set the value of the internal variable $_exitsignal,
-		 which holds the signal uncaught by the inferior.  */
-	      set_internalvar_integer (lookup_internalvar ("_exitsignal"),
-				       gdbarch_gdb_signal_to_target (gdbarch,
-							  ecs->ws.value.sig));
-	    }
-	  else
-	    {
-	      /* We don't have access to the target's method used for
-		 converting between signal numbers (GDB's internal
-		 representation <-> target's representation).
-		 Therefore, we cannot do a good job at displaying this
-		 information to the user.  It's better to just warn
-		 her about it (if infrun debugging is enabled), and
-		 give up.  */
-	      if (debug_infrun)
-		fprintf_filtered (gdb_stdlog, _("\
-Cannot fill $_exitsignal with the correct signal number.\n"));
-	    }
-
-	  gdb::observers::signal_exited.notify (ecs->ws.value.sig);
-	}
-
-      gdb_flush (gdb_stdout);
-      target_mourn_inferior (inferior_ptid);
+      handle_inferior_exit (ecs->ptid, ecs->ws);
       stop_print_frame = 0;
       stop_waiting (ecs);
       return;