diff mbox

[review] Introduce fetch_pending_stop

Message ID gerrit.1572371871000.I330cb8bda61ff9460f22d9e0eaaa96740d256202@gnutoolchain-gerrit.osci.io
State New, archived
Headers

Commit Message

Simon Marchi (Code Review) Oct. 29, 2019, 5:57 p.m. UTC 
  Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/427
......................................................................

Introduce fetch_pending_stop

This introduces a new "fetch_pending_stop" function and changes gdb to
use it.  This function removes the first matching pending stop from
the list of such stops.

Change-Id: I2d289dbf3ab17ae5c13c59afdf89b724301e962f

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

	* windows-nat.c (get_windows_debug_event): Use
	fetch_pending_stop.
	* nat/windows-nat.h (fetch_pending_stop): Declare.
	* nat/windows-nat.c (fetch_pending_stop): New function.

Change-Id: I330cb8bda61ff9460f22d9e0eaaa96740d256202
---
M gdb/ChangeLog
M gdb/nat/windows-nat.c
M gdb/nat/windows-nat.h
M gdb/windows-nat.c
4 files changed, 49 insertions(+), 20 deletions(-)

Comments

Simon Marchi (Code Review) Nov. 21, 2019, 6:40 p.m. UTC | #1 
Pedro Alves has posted comments on this change.

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


Patch Set 1: Code-Review+2

(1 comment)

LGTM.

| --- gdb/nat/windows-nat.c
| +++ gdb/nat/windows-nat.c
| @@ -315,10 +315,21 @@ matching_pending_stop (bool debug_events)
|  			 desired_stop_thread_id, item.thread_id));
|  	  return true;
|  	}
|      }
|  
|    return false;
|  }
|  
|  /* See nat/windows-nat.h.  */
|  
| +bool
| +fetch_pending_stop (bool debug_events, pending_stop *result)

PS1, Line 326:

I guess this could also be

 gdb::optional<pending_stop> fetch_pending_stop (bool debug_events, pending_stop *result)

Just mentioning it for completeness, what you have is fine.

| +{
| +  for (auto iter = pending_stops.begin ();
| +       iter != pending_stops.end ();
| +       ++iter)
| +    {
| +      if (desired_stop_thread_id == -1
| +	  || desired_stop_thread_id == iter->thread_id)
| +	{
| +	  *result = *iter;
Tom Tromey Nov. 21, 2019, 9:52 p.m. UTC | #2 
Pedro> | +bool
Pedro> | +fetch_pending_stop (bool debug_events, pending_stop *result)

Pedro> PS1, Line 326:

Pedro> I guess this could also be

Pedro>  gdb::optional<pending_stop> fetch_pending_stop (bool debug_events, pending_stop *result)

Pedro> Just mentioning it for completeness, what you have is fine.

I went ahead & did this.

Tom
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0d38851..e4bdd16 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@ 
 2019-10-29  Tom Tromey  <tromey@adacore.com>
 
+	* windows-nat.c (get_windows_debug_event): Use
+	fetch_pending_stop.
+	* nat/windows-nat.h (fetch_pending_stop): Declare.
+	* nat/windows-nat.c (fetch_pending_stop): New function.
+
+2019-10-29  Tom Tromey  <tromey@adacore.com>
+
 	* windows-nat.c (windows_continue): Use matching_pending_stop and
 	continue_last_debug_event.
 	* nat/windows-nat.h (matching_pending_stop)
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 163e3b1..7ac06f4 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -322,6 +322,33 @@ 
 
 /* See nat/windows-nat.h.  */
 
+bool
+fetch_pending_stop (bool debug_events, pending_stop *result)
+{
+  for (auto iter = pending_stops.begin ();
+       iter != pending_stops.end ();
+       ++iter)
+    {
+      if (desired_stop_thread_id == -1
+	  || desired_stop_thread_id == iter->thread_id)
+	{
+	  *result = *iter;
+	  current_event = iter->event;
+
+	  DEBUG_EVENTS (("get_windows_debug_event - "
+			 "pending stop found in 0x%x (desired=0x%x)\n",
+			 iter->thread_id, desired_stop_thread_id));
+
+	  pending_stops.erase (iter);
+	  return true;
+	}
+    }
+
+  return false;
+}
+
+/* See nat/windows-nat.h.  */
+
 BOOL
 continue_last_debug_event (DWORD continue_status, bool debug_events)
 {
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 42e2e75..2a5d4a1 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -216,6 +216,12 @@ 
 
 extern bool matching_pending_stop (bool debug_events);
 
+/* See if a pending stop matches DESIRED_STOP_THREAD_ID.  If so,
+   remove it from the list of pending stops, store it in *RESULT, set
+   'current_event', and return true.  Otherwise, return false.  */
+
+extern bool fetch_pending_stop (bool debug_events, pending_stop *result);
+
 /* A simple wrapper for ContinueDebugEvent that continues the last
    waited-for event.  */
 
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 6884411..e91e052 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1301,29 +1301,18 @@ 
   /* If there is a relevant pending stop, report it now.  See the
      comment by the definition of "pending_stops" for details on why
      this is needed.  */
-  for (auto iter = pending_stops.begin ();
-       iter != pending_stops.end ();
-       ++iter)
+  pending_stop stop;
+  if (fetch_pending_stop (debug_events, &stop))
     {
-      if (desired_stop_thread_id == -1
-	  || desired_stop_thread_id == iter->thread_id)
-	{
-	  thread_id = iter->thread_id;
-	  *ourstatus = iter->status;
-	  current_event = iter->event;
+      thread_id = stop.thread_id;
+      *ourstatus = stop.status;
 
-	  inferior_ptid = ptid_t (current_event.dwProcessId, thread_id, 0);
-	  current_windows_thread = thread_rec (inferior_ptid,
-					       INVALIDATE_CONTEXT);
-	  current_windows_thread->reload_context = 1;
+      inferior_ptid = ptid_t (current_event.dwProcessId, thread_id, 0);
+      current_windows_thread = thread_rec (inferior_ptid,
+					   INVALIDATE_CONTEXT);
+      current_windows_thread->reload_context = 1;
 
-	  DEBUG_EVENTS (("get_windows_debug_event - "
-			 "pending stop found in 0x%x (desired=0x%x)\n",
-			 thread_id, desired_stop_thread_id));
-
-	  pending_stops.erase (iter);
-	  return thread_id;
-	}
+      return thread_id;
     }
 
   last_sig = GDB_SIGNAL_0;