[PATCH 3/3] Remove find_inferior usage for thread_search

[PATCH 3/3] Remove find_inferior usage for thread_search

[Simon Marchi]

From: Simon Marchi <simon.marchi@ericsson.com>

Replace it with for_each_thread.  While at it, we can inline the
callback code.  One little change is that I am using the
prev_general_thread variable instead of current_gen_ptid, since they
should have the same value.

gdb/gdbserver/ChangeLog:

	* target.c (struct thread_search): Remove.
	(thread_search_callback): Remove.
	(prepare_to_access_memory): Use for_each_thread instead of
	find_inferior.  Inline code from thread_search_callback.
---
 gdb/gdbserver/target.c | 88 ++++++++++++++++++--------------------------------
 1 file changed, 32 insertions(+), 56 deletions(-)

diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index 8f757c0a5d..204f941f20 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -32,51 +32,6 @@ set_desired_thread ()
   return (current_thread != NULL);
 }
 
-/* Structure used to look up a thread to use as current when accessing
-   memory.  */
-
-struct thread_search
-{
-  /* The PTID of the current general thread.  This is an input
-     parameter.  */
-  ptid_t current_gen_ptid;
-
-  /* The first thread found.  */
-  struct thread_info *first;
-
-  /* The first stopped thread found.  */
-  struct thread_info *stopped;
-
-  /* The current general thread, if found.  */
-  struct thread_info *current;
-};
-
-/* Callback for find_inferior.  Search for a thread to use as current
-   when accessing memory.  */
-
-static int
-thread_search_callback (thread_info *thread, void *args)
-{
-  struct thread_search *s = (struct thread_search *) args;
-
-  if (thread->id.pid () == ptid_get_pid (s->current_gen_ptid)
-      && mythread_alive (ptid_of (thread)))
-    {
-      if (s->stopped == NULL
-	  && the_target->thread_stopped != NULL
-	  && thread_stopped (thread))
-	s->stopped = thread;
-
-      if (s->first == NULL)
-	s->first = thread;
-
-      if (s->current == NULL && s->current_gen_ptid == thread->id)
-	s->current = thread;
-    }
-
-  return 0;
-}
-
 /* The thread that was current before prepare_to_access_memory was
    called.  done_accessing_memory uses this to restore the previous
    selected thread.  */
@@ -87,11 +42,15 @@ static ptid_t prev_general_thread;
 int
 prepare_to_access_memory (void)
 {
-  struct thread_search search;
-  struct thread_info *thread;
+  /* The first thread found.  */
+  struct thread_info *first = NULL;
+  /* The first stopped thread found.  */
+  struct thread_info *stopped = NULL;
+  /* The current general thread, if found.  */
+  struct thread_info *current = NULL;
 
-  memset (&search, 0, sizeof (search));
-  search.current_gen_ptid = general_thread;
+  /* Save the general thread value, since prepare_to_access_memory could change
+     it.  */
   prev_general_thread = general_thread;
 
   if (the_target->prepare_to_access_memory != NULL)
@@ -103,18 +62,35 @@ prepare_to_access_memory (void)
 	return res;
     }
 
-  find_inferior (&all_threads, thread_search_callback, &search);
+  for_each_thread (prev_general_thread.pid (), [&] (thread_info *thread)
+    {
+      if (mythread_alive (thread->id))
+	{
+	  if (stopped == NULL && the_target->thread_stopped != NULL
+	      && thread_stopped (thread))
+	    stopped = thread;
+
+	  if (first == NULL)
+	    first = thread;
+
+	  if (current == NULL && prev_general_thread == thread->id)
+	    current = thread;
+	}
+    });
+
+  /* The thread we end up choosing.  */
+  struct thread_info *thread;
 
   /* Prefer a stopped thread.  If none is found, try the current
      thread.  Otherwise, take the first thread in the process.  If
      none is found, undo the effects of
      target->prepare_to_access_memory() and return error.  */
-  if (search.stopped != NULL)
-    thread = search.stopped;
-  else if (search.current != NULL)
-    thread = search.current;
-  else if (search.first != NULL)
-    thread = search.first;
+  if (stopped != NULL)
+    thread = stopped;
+  else if (current != NULL)
+    thread = current;
+  else if (first != NULL)
+    thread = first;
   else
     {
       done_accessing_memory ();
-- 
2.14.2

[PATCH 2/3] Remove usage of find_inferior in resume

[Simon Marchi]

From: Simon Marchi <simon.marchi@ericsson.com>

Change find_inferior with find_thread.  Since we can now pass arguments
directly instead of through a void pointer, we don't need the
visit_actioned_threads_data structure anymore.

gdb/gdbserver/ChangeLog:

	* server.c (struct visit_actioned_threads_data): Remove.
	(visit_actioned_threads): Change prototype to take arguments
	directly.
	(resume): Use find_thread instead of find_inferior.
---
 gdb/gdbserver/server.c | 43 ++++++++++++++++---------------------------
 1 file changed, 16 insertions(+), 27 deletions(-)

diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 650cf1c674..e827b9c701 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2677,31 +2677,18 @@ static void resume (struct thread_resume *actions, size_t n);
 typedef int (visit_actioned_threads_callback_ftype)
   (const struct thread_resume *, struct thread_info *);
 
-/* Struct to pass data to visit_actioned_threads.  */
-
-struct visit_actioned_threads_data
-{
-  const struct thread_resume *actions;
-  size_t num_actions;
-  visit_actioned_threads_callback_ftype *callback;
-};
-
 /* Call CALLBACK for any thread to which ACTIONS applies to.  Returns
    true if CALLBACK returns true.  Returns false if no matching thread
    is found or CALLBACK results false.
-   Note: This function is itself a callback for find_inferior.  */
+   Note: This function is itself a callback for find_thread.  */
 
-static int
-visit_actioned_threads (thread_info *thread, void *datap)
+static bool
+visit_actioned_threads (thread_info *thread,
+			const struct thread_resume *actions,
+			size_t num_actions,
+			visit_actioned_threads_callback_ftype *callback)
 {
-  struct visit_actioned_threads_data *data
-    = (struct visit_actioned_threads_data *) datap;
-  const struct thread_resume *actions = data->actions;
-  size_t num_actions = data->num_actions;
-  visit_actioned_threads_callback_ftype *callback = data->callback;
-  size_t i;
-
-  for (i = 0; i < num_actions; i++)
+  for (size_t i = 0; i < num_actions; i++)
     {
       const struct thread_resume *action = &actions[i];
 
@@ -2712,11 +2699,11 @@ visit_actioned_threads (thread_info *thread, void *datap)
 	      && ptid_get_lwp (action->thread) == -1))
 	{
 	  if ((*callback) (action, thread))
-	    return 1;
+	    return true;
 	}
     }
 
-  return 0;
+  return false;
 }
 
 /* Callback for visit_actioned_threads.  If the thread has a pending
@@ -2858,12 +2845,14 @@ resume (struct thread_resume *actions, size_t num_actions)
 	 one with a pending status to report.  If so, skip actually
 	 resuming/stopping and report the pending event
 	 immediately.  */
-      struct visit_actioned_threads_data data;
 
-      data.actions = actions;
-      data.num_actions = num_actions;
-      data.callback = handle_pending_status;
-      if (find_inferior (&all_threads, visit_actioned_threads, &data) != NULL)
+      thread_info *thread_with_status = find_thread ([&] (thread_info *thread)
+	{
+	  return visit_actioned_threads (thread, actions, num_actions,
+					 handle_pending_status);
+	});
+
+      if (thread_with_status != NULL)
 	return;
 
       enable_async_io ();
-- 
2.14.2

[PATCH 1/3] Remove usages of find_inferior in handle_status

[Simon Marchi]

From: Simon Marchi <simon.marchi@ericsson.com>

Replace one with find_thread, the other with for_each_thread.

gdb/gdbserver/ChangeLog:

	* server.c (queue_stop_reply_callback): Change prototype, return
	void.
	(find_status_pending_thread_callback): Remove.
	(handle_status): Replace find_inferior with find_thread and
	for_each_thread.
---
 gdb/gdbserver/server.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 892d765714..650cf1c674 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -3230,8 +3230,8 @@ myresume (char *own_buf, int step, int sig)
 /* Callback for for_each_inferior.  Make a new stop reply for each
    stopped thread.  */
 
-static int
-queue_stop_reply_callback (thread_info *thread, void *arg)
+static void
+queue_stop_reply_callback (thread_info *thread)
 {
   /* For now, assume targets that don't have this callback also don't
      manage the thread's last_status field.  */
@@ -3267,8 +3267,6 @@ queue_stop_reply_callback (thread_info *thread, void *arg)
 	  queue_stop_reply (thread->id, &thread->last_status);
 	}
     }
-
-  return 0;
 }
 
 /* Set this inferior threads's state as "want-stopped".  We won't
@@ -3324,15 +3322,6 @@ set_pending_status_callback (thread_info *thread)
     thread->status_pending_p = 1;
 }
 
-/* Callback for find_inferior.  Return true if ENTRY (a thread) has a
-   pending status to report to GDB.  */
-
-static int
-find_status_pending_thread_callback (thread_info *thread, void *data)
-{
-  return thread->status_pending_p;
-}
-
 /* Status handler for the '?' packet.  */
 
 static void
@@ -3349,7 +3338,7 @@ handle_status (char *own_buf)
 
   if (non_stop)
     {
-      find_inferior (&all_threads, queue_stop_reply_callback, NULL);
+      for_each_thread (queue_stop_reply_callback);
 
       /* The first is sent immediatly.  OK is sent if there is no
 	 stopped thread, which is the same handling of the vStopped
@@ -3382,8 +3371,10 @@ handle_status (char *own_buf)
       /* If the last event thread is not found for some reason, look
 	 for some other thread that might have an event to report.  */
       if (thread == NULL)
-	thread = find_inferior (&all_threads,
-				find_status_pending_thread_callback, NULL);
+	thread = find_thread ([] (thread_info *thread)
+	  {
+	    return thread->status_pending_p;
+	  });
 
       /* If we're still out of luck, simply pick the first thread in
 	 the thread list.  */
-- 
2.14.2

Re: [PATCH 1/3] Remove usages of find_inferior in handle_status

[Simon Marchi]

On 2017-10-21 12:37, Simon Marchi wrote:
> From: Simon Marchi <simon.marchi@ericsson.com>
> 
> Replace one with find_thread, the other with for_each_thread.
> 
> gdb/gdbserver/ChangeLog:
> 
> 	* server.c (queue_stop_reply_callback): Change prototype, return
> 	void.
> 	(find_status_pending_thread_callback): Remove.
> 	(handle_status): Replace find_inferior with find_thread and
> 	for_each_thread.

I pushed these patches in.

Simon