[v3,1/3] gdbserver: use 'gdb::function_view' in 'find_*' and 'for_each_*'

Message ID 20241018152052.1509373-2-stephan.rohr@intel.com
State New
Headers
Series gdbserver: add thread map |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed

Commit Message

Rohr, Stephan Oct. 18, 2024, 3:20 p.m. UTC
  Remove the templated versions of 'find_thread', 'for_each_thread' and
'find_thread_in_random' and replace the template function argument with
'gdb::function_view'.
---
 gdbserver/gdbthread.h  | 101 ++++--------------------------
 gdbserver/inferiors.cc | 138 +++++++++++++++++++++++++++++++++++++++++
 gdbserver/inferiors.h  |  36 +----------
 3 files changed, 154 insertions(+), 121 deletions(-)
  

Comments

Simon Marchi Oct. 18, 2024, 5:34 p.m. UTC | #1
On 2024-10-18 11:20, Stephan Rohr wrote:
> Remove the templated versions of 'find_thread', 'for_each_thread' and
> 'find_thread_in_random' and replace the template function argument with
> 'gdb::function_view'.

It would perhaps be good to say why we do that change.  It's a bit
subjective, but in my opinion function_view is easier to work with than
templates.  The function_view type documents well the types of the
parameters taken by the callback and its return type.  The error
messages are less cryptic, when there are errors.

> -template <typename Func>
> -static process_info *
> -find_process (Func func)
> -{
> -  std::list<process_info *>::iterator next, cur = all_processes.begin ();
> -
> -  while (cur != all_processes.end ())
> -    {
> -      next = cur;
> -      next++;
> -
> -      if (func (*cur))
> -	return *cur;
> -
> -      cur = next;
> -    }
> -
> -  return NULL;
> -}
> +process_info *
> +find_process (gdb::function_view<bool (process_info *)> func);

Remove line break after return type.

Otherwise, this patch LGTM:

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon
  

Patch

diff --git a/gdbserver/gdbthread.h b/gdbserver/gdbthread.h
index c5a5498088f..adc5857abfd 100644
--- a/gdbserver/gdbthread.h
+++ b/gdbserver/gdbthread.h
@@ -20,6 +20,7 @@ 
 #define GDBSERVER_GDBTHREAD_H
 
 #include "gdbsupport/common-gdbthread.h"
+#include "gdbsupport/function-view.h"
 #include "inferiors.h"
 
 #include <list>
@@ -103,111 +104,35 @@  struct thread_info *find_any_thread_of_pid (int pid);
 /* Find the first thread for which FUNC returns true.  Return NULL if no thread
    satisfying FUNC is found.  */
 
-template <typename Func>
-static thread_info *
-find_thread (Func func)
-{
-  std::list<thread_info *>::iterator next, cur = all_threads.begin ();
-
-  while (cur != all_threads.end ())
-    {
-      next = cur;
-      next++;
-
-      if (func (*cur))
-	return *cur;
-
-      cur = next;
-    }
-
-  return NULL;
-}
+thread_info *
+find_thread (gdb::function_view<bool (thread_info *)> func);
 
 /* Like the above, but only consider threads with pid PID.  */
 
-template <typename Func>
-static thread_info *
-find_thread (int pid, Func func)
-{
-  return find_thread ([&] (thread_info *thread)
-    {
-      return thread->id.pid () == pid && func (thread);
-    });
-}
+thread_info *
+find_thread (int pid, gdb::function_view<bool (thread_info *)> func);
 
 /* Find the first thread that matches FILTER for which FUNC returns true.
    Return NULL if no thread satisfying these conditions is found.  */
 
-template <typename Func>
-static thread_info *
-find_thread (ptid_t filter, Func func)
-{
-  return find_thread ([&] (thread_info *thread) {
-    return thread->id.matches (filter) && func (thread);
-  });
-}
+thread_info *
+find_thread (ptid_t filter, gdb::function_view<bool (thread_info *)> func);
 
 /* Invoke FUNC for each thread.  */
 
-template <typename Func>
-static void
-for_each_thread (Func func)
-{
-  std::list<thread_info *>::iterator next, cur = all_threads.begin ();
-
-  while (cur != all_threads.end ())
-    {
-      next = cur;
-      next++;
-      func (*cur);
-      cur = next;
-    }
-}
+void
+for_each_thread (gdb::function_view<void (thread_info *)> func);
 
 /* Like the above, but only consider threads with pid PID.  */
 
-template <typename Func>
-static void
-for_each_thread (int pid, Func func)
-{
-  for_each_thread ([&] (thread_info *thread)
-    {
-      if (pid == thread->id.pid ())
-	func (thread);
-    });
-}
+void
+for_each_thread (int pid, gdb::function_view<void (thread_info *)> func);
 
 /* Find the a random thread for which FUNC (THREAD) returns true.  If
    no entry is found then return NULL.  */
 
-template <typename Func>
-static thread_info *
-find_thread_in_random (Func func)
-{
-  int count = 0;
-  int random_selector;
-
-  /* First count how many interesting entries we have.  */
-  for_each_thread ([&] (thread_info *thread) {
-    if (func (thread))
-      count++;
-  });
-
-  if (count == 0)
-    return NULL;
-
-  /* Now randomly pick an entry out of those.  */
-  random_selector = (int)
-    ((count * (double) rand ()) / (RAND_MAX + 1.0));
-
-  thread_info *thread = find_thread ([&] (thread_info *thr_arg) {
-    return func (thr_arg) && (random_selector-- == 0);
-  });
-
-  gdb_assert (thread != NULL);
-
-  return thread;
-}
+thread_info *
+find_thread_in_random (gdb::function_view<bool (thread_info *)> func);
 
 /* Get current thread ID (Linux task ID).  */
 #define current_ptid (current_thread->id)
diff --git a/gdbserver/inferiors.cc b/gdbserver/inferiors.cc
index d088340474f..5621db377fb 100644
--- a/gdbserver/inferiors.cc
+++ b/gdbserver/inferiors.cc
@@ -215,6 +215,144 @@  current_process (void)
   return current_process_;
 }
 
+/* See inferiors.h.  */
+
+void
+for_each_process (gdb::function_view<void (process_info *)> func)
+{
+  std::list<process_info *>::iterator next, cur = all_processes.begin ();
+
+  while (cur != all_processes.end ())
+    {
+      next = cur;
+      next++;
+      func (*cur);
+      cur = next;
+    }
+}
+
+/* See inferiors.h.  */
+
+process_info *
+find_process (gdb::function_view<bool (process_info *)> func)
+{
+  std::list<process_info *>::iterator next, cur = all_processes.begin ();
+
+  while (cur != all_processes.end ())
+    {
+      next = cur;
+      next++;
+
+      if (func (*cur))
+	return *cur;
+
+      cur = next;
+    }
+
+  return NULL;
+}
+
+/* See gdbthread.h.  */
+
+thread_info *
+find_thread (gdb::function_view<bool (thread_info *)> func)
+{
+  std::list<thread_info *>::iterator next, cur = all_threads.begin ();
+
+  while (cur != all_threads.end ())
+    {
+      next = cur;
+      next++;
+
+      if (func (*cur))
+	return *cur;
+
+      cur = next;
+    }
+
+  return NULL;
+}
+
+/* See gdbthread.h.  */
+
+thread_info *
+find_thread (int pid, gdb::function_view<bool (thread_info *)> func)
+{
+  return find_thread ([&] (thread_info *thread)
+    {
+      return thread->id.pid () == pid && func (thread);
+    });
+}
+
+/* See gdbthread.h.  */
+
+thread_info *
+find_thread (ptid_t filter, gdb::function_view<bool (thread_info *)> func)
+{
+  return find_thread ([&] (thread_info *thread) {
+    return thread->id.matches (filter) && func (thread);
+  });
+}
+
+/* See gdbthread.h.  */
+
+void
+for_each_thread (gdb::function_view<void (thread_info *)> func)
+{
+  std::list<thread_info *>::iterator next, cur = all_threads.begin ();
+
+  while (cur != all_threads.end ())
+    {
+      next = cur;
+      next++;
+      func (*cur);
+      cur = next;
+    }
+}
+
+/* See gdbthread.h.  */
+
+void
+for_each_thread (int pid, gdb::function_view<void (thread_info *)> func)
+{
+  for_each_thread ([&] (thread_info *thread)
+    {
+      if (pid == thread->id.pid ())
+	func (thread);
+    });
+}
+
+/* See gdbthread.h.  */
+
+thread_info *
+find_thread_in_random (gdb::function_view<bool (thread_info *)> func)
+{
+  int count = 0;
+  int random_selector;
+
+  /* First count how many interesting entries we have.  */
+  for_each_thread ([&] (thread_info *thread) {
+    if (func (thread))
+      count++;
+  });
+
+  if (count == 0)
+    return NULL;
+
+  /* Now randomly pick an entry out of those.  */
+  random_selector = (int)
+    ((count * (double) rand ()) / (RAND_MAX + 1.0));
+
+  thread_info *thread = find_thread ([&] (thread_info *thr_arg) {
+    return func (thr_arg) && (random_selector-- == 0);
+  });
+
+  gdb_assert (thread != NULL);
+
+  return thread;
+}
+
+
 /* See gdbsupport/common-gdbthread.h.  */
 
 void
diff --git a/gdbserver/inferiors.h b/gdbserver/inferiors.h
index 00e42335014..5f865fa860a 100644
--- a/gdbserver/inferiors.h
+++ b/gdbserver/inferiors.h
@@ -103,43 +103,13 @@  extern std::list<process_info *> all_processes;
 
 /* Invoke FUNC for each process.  */
 
-template <typename Func>
-static void
-for_each_process (Func func)
-{
-  std::list<process_info *>::iterator next, cur = all_processes.begin ();
-
-  while (cur != all_processes.end ())
-    {
-      next = cur;
-      next++;
-      func (*cur);
-      cur = next;
-    }
-}
+void for_each_process (gdb::function_view<void (process_info *)> func);
 
 /* Find the first process for which FUNC returns true.  Return NULL if no
    process satisfying FUNC is found.  */
 
-template <typename Func>
-static process_info *
-find_process (Func func)
-{
-  std::list<process_info *>::iterator next, cur = all_processes.begin ();
-
-  while (cur != all_processes.end ())
-    {
-      next = cur;
-      next++;
-
-      if (func (*cur))
-	return *cur;
-
-      cur = next;
-    }
-
-  return NULL;
-}
+process_info *
+find_process (gdb::function_view<bool (process_info *)> func);
 
 extern struct thread_info *current_thread;