Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH 0/3] Get rid of inferior_list
@ 2017-10-09 14:30 Simon Marchi
  2017-10-09 14:30 ` [PATCH 2/3] gdbserver: Use std::list for all_processes Simon Marchi
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Simon Marchi @ 2017-10-09 14:30 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

This patchset removes the inferior_list/inferior_list_entry structures,
in favor of using std::list.  When working in gdbserver, I find it a bit
cumbersome to work with the threads and processes list, having to cast
from inferior_list_entry to the real type.

Simon Marchi (3):
  gdbserver: Use std::list for all_dlls
  gdbserver: Use std::list for all_processes
  gdbserver: use std::list for all_threads

 gdb/gdbserver/dll.c            |  36 ++---
 gdb/gdbserver/dll.h            |   8 +-
 gdb/gdbserver/gdbthread.h      | 109 ++++++++++++++-
 gdb/gdbserver/inferiors.c      | 301 +++++++++++------------------------------
 gdb/gdbserver/inferiors.h      | 124 ++++++++---------
 gdb/gdbserver/linux-arm-low.c  |   5 +-
 gdb/gdbserver/linux-low.c      | 277 ++++++++++++++++---------------------
 gdb/gdbserver/linux-mips-low.c |   4 +-
 gdb/gdbserver/linux-x86-low.c  |  23 ++--
 gdb/gdbserver/lynx-low.c       |  10 +-
 gdb/gdbserver/regcache.c       |  18 +--
 gdb/gdbserver/server.c         | 197 +++++++++------------------
 gdb/gdbserver/target.c         |   7 +-
 gdb/gdbserver/thread-db.c      |   4 +-
 gdb/gdbserver/tracepoint.c     |   8 +-
 gdb/gdbserver/win32-i386-low.c |   4 +-
 gdb/gdbserver/win32-low.c      |  16 +--
 17 files changed, 479 insertions(+), 672 deletions(-)

-- 
2.14.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/3] gdbserver: Use std::list for all_processes
  2017-10-09 14:30 [PATCH 0/3] Get rid of inferior_list Simon Marchi
@ 2017-10-09 14:30 ` Simon Marchi
  2017-10-09 14:30 ` [PATCH 3/3] gdbserver: use std::list for all_threads Simon Marchi
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Simon Marchi @ 2017-10-09 14:30 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

Remove the usage of inferior_list for the all_processes list in
gdbserver, replace it with an std::list. The entry field in process_info
is removed, and replaced by a simple pid field.

The pid_of macro, used for both processes and threads, is replaced with
separate functions.  For completeness, I changed ptid_of and lwpid_of to
functions as well.

gdb/gdbserver/ChangeLog:

	* gdbthread.h (ptid_of, pid_of, lwpid_of): New functions.
	* inferiors.h: Include <list>.
	(struct process_info) <entry>: Remove field.
	<pid>: New field.
	(pid_of): Change macro to function.
	(ptid_of, lwpid_of): Remove macro.
	(all_processes): Change type to std::list<process_info *>.
	(ALL_PROCESSES): Remove macro.
	(for_each_process, find_process): New function.
	* inferiors.c (all_processes): Change type to
	std::list<process_info *>.
	(find_thread_process): Adjust.
	(add_process): Likewise.
	(remove_process): Likewise.
	(find_process_pid): Likewise.
	(get_first_process): Likewise.
	(started_inferior_callback): Remove.
	(have_started_inferiors_p): Adjust.
	(attached_inferior_callback): Remove.
	(have_attached_inferiors_p): Adjust.
	* linux-low.c (check_zombie_leaders): Likewise.
	* linux-x86-low.c (x86_arch_setup_process_callback): Remove.
	(x86_linux_update_xmltarget): Adjust.
	* server.c (handle_query): Likewise.
	(gdb_reattached_process): Remove.
	(handle_status): Adjust.
	(kill_inferior_callback): Likewise.
	(detach_or_kill_inferior): Remove.
	(print_started_pid): Likewise.
	(print_attached_pid): Likewise.
	(detach_or_kill_for_exit): Update.
	(process_serial_event): Likewise.
	* linux-arm-low.c (arm_new_fork): Likewise.
---
 gdb/gdbserver/gdbthread.h     |  24 +++++++++
 gdb/gdbserver/inferiors.c     |  57 ++++++++--------------
 gdb/gdbserver/inferiors.h     |  60 +++++++++++++++++++----
 gdb/gdbserver/linux-arm-low.c |   2 +-
 gdb/gdbserver/linux-low.c     | 111 ++++++++++++++++++++----------------------
 gdb/gdbserver/linux-x86-low.c |  23 +++------
 gdb/gdbserver/server.c        | 102 ++++++++++++--------------------------
 7 files changed, 189 insertions(+), 190 deletions(-)

diff --git a/gdb/gdbserver/gdbthread.h b/gdb/gdbserver/gdbthread.h
index a864f95884..93688a36e4 100644
--- a/gdb/gdbserver/gdbthread.h
+++ b/gdb/gdbserver/gdbthread.h
@@ -88,6 +88,30 @@ struct thread_info *find_any_thread_of_pid (int pid);
 /* Get current thread ID (Linux task ID).  */
 #define current_ptid (current_thread->entry.id)
 
+/* Get the ptid of THREAD.  */
+
+static inline ptid_t
+ptid_of (const thread_info *thread)
+{
+  return thread->entry.id;
+}
+
+/* Get the pid of THREAD.  */
+
+static inline int
+pid_of (const thread_info *thread)
+{
+  return thread->entry.id.pid ();
+}
+
+/* Get the lwp of THREAD.  */
+
+static inline long
+lwpid_of (const thread_info *thread)
+{
+  return thread->entry.id.lwp ();
+}
+
 /* Create a cleanup to restore current_thread.  */
 struct cleanup *make_cleanup_restore_current_thread (void);
 
diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
index 154c167f4c..13ee8c97eb 100644
--- a/gdb/gdbserver/inferiors.c
+++ b/gdb/gdbserver/inferiors.c
@@ -22,7 +22,7 @@
 #include "gdbthread.h"
 #include "dll.h"
 
-struct inferior_list all_processes;
+std::list<process_info *> all_processes;
 struct inferior_list all_threads;
 
 struct thread_info *current_thread;
@@ -144,7 +144,7 @@ find_thread_ptid (ptid_t ptid)
 static struct thread_info *
 find_thread_process (const struct process_info *const process)
 {
-  return find_any_thread_of_pid (process->entry.id.pid ());
+  return find_any_thread_of_pid (process->pid);
 }
 
 /* Helper for find_any_thread_of_pid.  Returns true if a thread
@@ -336,10 +336,10 @@ add_process (int pid, int attached)
 {
   struct process_info *process = XCNEW (struct process_info);
 
-  process->entry.id = pid_to_ptid (pid);
+  process->pid = pid;
   process->attached = attached;
 
-  add_inferior_to_list (&all_processes, &process->entry);
+  all_processes.push_back (process);
 
   return process;
 }
@@ -354,35 +354,28 @@ remove_process (struct process_info *process)
   clear_symbol_cache (&process->symbol_cache);
   free_all_breakpoints (process);
   gdb_assert (find_thread_process (process) == NULL);
-  remove_inferior (&all_processes, &process->entry);
+  all_processes.remove (process);
   VEC_free (int, process->syscalls_to_catch);
   free (process);
 }
 
-struct process_info *
+process_info *
 find_process_pid (int pid)
 {
-  return (struct process_info *)
-    find_inferior_id (&all_processes, pid_to_ptid (pid));
+  return find_process ([&] (process_info *process) {
+    return process->pid == pid;
+  });
 }
 
-/* Wrapper around get_first_inferior to return a struct process_info *.  */
+/* Get the first process in the process list, or NULL if the list is empty.  */
 
-struct process_info *
+process_info *
 get_first_process (void)
 {
-  return (struct process_info *) get_first_inferior (&all_processes);
-}
-
-/* Return non-zero if INF, a struct process_info, was started by us,
-   i.e. not attached to.  */
-
-static int
-started_inferior_callback (struct inferior_list_entry *entry, void *args)
-{
-  struct process_info *process = (struct process_info *) entry;
-
-  return ! process->attached;
+  if (!all_processes.empty ())
+    return all_processes.front ();
+  else
+    return NULL;
 }
 
 /* Return non-zero if there are any inferiors that we have created
@@ -391,18 +384,9 @@ started_inferior_callback (struct inferior_list_entry *entry, void *args)
 int
 have_started_inferiors_p (void)
 {
-  return (find_inferior (&all_processes, started_inferior_callback, NULL)
-	  != NULL);
-}
-
-/* Return non-zero if INF, a struct process_info, was attached to.  */
-
-static int
-attached_inferior_callback (struct inferior_list_entry *entry, void *args)
-{
-  struct process_info *process = (struct process_info *) entry;
-
-  return process->attached;
+  return find_process ([] (process_info *process) {
+    return !process->attached;
+  }) != NULL;
 }
 
 /* Return non-zero if there are any inferiors that we have attached to.  */
@@ -410,8 +394,9 @@ attached_inferior_callback (struct inferior_list_entry *entry, void *args)
 int
 have_attached_inferiors_p (void)
 {
-  return (find_inferior (&all_processes, attached_inferior_callback, NULL)
-	  != NULL);
+  return find_process ([] (process_info *process) {
+    return process->attached;
+  }) != NULL;
 }
 
 struct process_info *
diff --git a/gdb/gdbserver/inferiors.h b/gdb/gdbserver/inferiors.h
index aef8433980..674b8c6dcb 100644
--- a/gdb/gdbserver/inferiors.h
+++ b/gdb/gdbserver/inferiors.h
@@ -20,6 +20,7 @@
 #define INFERIORS_H
 
 #include "gdb_vecs.h"
+#include <list>
 
 /* Generic information for tracking a list of ``inferiors'' - threads,
    processes, etc.  */
@@ -45,9 +46,8 @@ struct process_info_private;
 
 struct process_info
 {
-  /* This must appear first.
-     The list iterator functions assume it.  */
-  struct inferior_list_entry entry;
+  /* This process' pid.  */
+  int pid;
 
   /* Nonzero if this child process was attached rather than
      spawned.  */
@@ -79,9 +79,13 @@ struct process_info
   struct process_info_private *priv;
 };
 
-#define ptid_of(inf) ((inf)->entry.id)
-#define pid_of(inf) ptid_get_pid ((inf)->entry.id)
-#define lwpid_of(inf) ptid_get_lwp ((inf)->entry.id)
+/* Get the pid of PROC.  */
+
+static inline int
+pid_of (const process_info *proc)
+{
+  return proc->pid;
+}
 
 /* Return a pointer to the process that corresponds to the current
    thread (current_thread).  It is an error to call this if there is
@@ -90,7 +94,7 @@ struct process_info
 struct process_info *current_process (void);
 struct process_info *get_thread_process (const struct thread_info *);
 
-extern struct inferior_list all_processes;
+extern std::list<process_info *> all_processes;
 
 void add_inferior_to_list (struct inferior_list *list,
 			   struct inferior_list_entry *new_inferior);
@@ -124,9 +128,45 @@ int one_inferior_p (struct inferior_list *list);
 #define ALL_INFERIORS(list, cur, tmp)				\
   ALL_INFERIORS_TYPE (struct inferior_list_entry, list, cur, tmp)
 
-/* Iterate over all processes, open loop style.  */
-#define ALL_PROCESSES(cur, tmp)					\
-  ALL_INFERIORS_TYPE (struct process_info, &all_processes, cur, tmp)
+/* 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;
+    }
+}
+
+/* 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;
+}
 
 extern struct thread_info *current_thread;
 void remove_inferior (struct inferior_list *list,
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index 5a3f465dee..cb59c2a130 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -691,7 +691,7 @@ arm_new_fork (struct process_info *parent, struct process_info *child)
 
   /* Mark all the hardware breakpoints and watchpoints as changed to
      make sure that the registers will be updated.  */
-  child_lwp = find_lwp_pid (ptid_of (child));
+  child_lwp = find_lwp_pid (ptid_t (child->pid));
   child_lwp_info = child_lwp->arch_private;
   for (i = 0; i < MAX_BPTS; i++)
     child_lwp_info->bpts_changed[i] = 1;
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 54005c1f1d..dc0ad85734 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -1936,63 +1936,60 @@ iterate_over_lwps (ptid_t filter,
 static void
 check_zombie_leaders (void)
 {
-  struct process_info *proc, *tmp;
-
-  ALL_PROCESSES (proc, tmp)
-    {
-      pid_t leader_pid = pid_of (proc);
-      struct lwp_info *leader_lp;
-
-      leader_lp = find_lwp_pid (pid_to_ptid (leader_pid));
-
-      if (debug_threads)
-	debug_printf ("leader_pid=%d, leader_lp!=NULL=%d, "
-		      "num_lwps=%d, zombie=%d\n",
-		      leader_pid, leader_lp!= NULL, num_lwps (leader_pid),
-		      linux_proc_pid_is_zombie (leader_pid));
-
-      if (leader_lp != NULL && !leader_lp->stopped
-	  /* Check if there are other threads in the group, as we may
-	     have raced with the inferior simply exiting.  */
-	  && !last_thread_of_process_p (leader_pid)
-	  && linux_proc_pid_is_zombie (leader_pid))
-	{
-	  /* A leader zombie can mean one of two things:
-
-	     - It exited, and there's an exit status pending
-	     available, or only the leader exited (not the whole
-	     program).  In the latter case, we can't waitpid the
-	     leader's exit status until all other threads are gone.
-
-	     - There are 3 or more threads in the group, and a thread
-	     other than the leader exec'd.  On an exec, the Linux
-	     kernel destroys all other threads (except the execing
-	     one) in the thread group, and resets the execing thread's
-	     tid to the tgid.  No exit notification is sent for the
-	     execing thread -- from the ptracer's perspective, it
-	     appears as though the execing thread just vanishes.
-	     Until we reap all other threads except the leader and the
-	     execing thread, the leader will be zombie, and the
-	     execing thread will be in `D (disc sleep)'.  As soon as
-	     all other threads are reaped, the execing thread changes
-	     it's tid to the tgid, and the previous (zombie) leader
-	     vanishes, giving place to the "new" leader.  We could try
-	     distinguishing the exit and exec cases, by waiting once
-	     more, and seeing if something comes out, but it doesn't
-	     sound useful.  The previous leader _does_ go away, and
-	     we'll re-add the new one once we see the exec event
-	     (which is just the same as what would happen if the
-	     previous leader did exit voluntarily before some other
-	     thread execs).  */
-
-	  if (debug_threads)
-	    debug_printf ("CZL: Thread group leader %d zombie "
-			  "(it exited, or another thread execd).\n",
-			  leader_pid);
-
-	  delete_lwp (leader_lp);
-	}
-    }
+  for_each_process ([] (process_info *proc) {
+    pid_t leader_pid = pid_of (proc);
+    struct lwp_info *leader_lp;
+
+    leader_lp = find_lwp_pid (pid_to_ptid (leader_pid));
+
+    if (debug_threads)
+      debug_printf ("leader_pid=%d, leader_lp!=NULL=%d, "
+		    "num_lwps=%d, zombie=%d\n",
+		    leader_pid, leader_lp!= NULL, num_lwps (leader_pid),
+		    linux_proc_pid_is_zombie (leader_pid));
+
+    if (leader_lp != NULL && !leader_lp->stopped
+	/* Check if there are other threads in the group, as we may
+	   have raced with the inferior simply exiting.  */
+	&& !last_thread_of_process_p (leader_pid)
+	&& linux_proc_pid_is_zombie (leader_pid))
+      {
+	/* A leader zombie can mean one of two things:
+
+	   - It exited, and there's an exit status pending
+	   available, or only the leader exited (not the whole
+	   program).  In the latter case, we can't waitpid the
+	   leader's exit status until all other threads are gone.
+
+	   - There are 3 or more threads in the group, and a thread
+	   other than the leader exec'd.  On an exec, the Linux
+	   kernel destroys all other threads (except the execing
+	   one) in the thread group, and resets the execing thread's
+	   tid to the tgid.  No exit notification is sent for the
+	   execing thread -- from the ptracer's perspective, it
+	   appears as though the execing thread just vanishes.
+	   Until we reap all other threads except the leader and the
+	   execing thread, the leader will be zombie, and the
+	   execing thread will be in `D (disc sleep)'.  As soon as
+	   all other threads are reaped, the execing thread changes
+	   it's tid to the tgid, and the previous (zombie) leader
+	   vanishes, giving place to the "new" leader.  We could try
+	   distinguishing the exit and exec cases, by waiting once
+	   more, and seeing if something comes out, but it doesn't
+	   sound useful.  The previous leader _does_ go away, and
+	   we'll re-add the new one once we see the exec event
+	   (which is just the same as what would happen if the
+	   previous leader did exit voluntarily before some other
+	   thread execs).  */
+
+	if (debug_threads)
+	  debug_printf ("CZL: Thread group leader %d zombie "
+			"(it exited, or another thread execd).\n",
+			leader_pid);
+
+	delete_lwp (leader_lp);
+      }
+    });
 }
 
 /* Callback for `find_inferior'.  Returns the first LWP that is not
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 49f6b2d489..0e5c8d48b7 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -846,20 +846,6 @@ x86_linux_read_description (void)
   gdb_assert_not_reached ("failed to return tdesc");
 }
 
-/* Callback for for_each_inferior.  Calls the arch_setup routine for
-   each process.  */
-
-static void
-x86_arch_setup_process_callback (struct inferior_list_entry *entry)
-{
-  int pid = ptid_get_pid (entry->id);
-
-  /* Look up any thread of this processes.  */
-  current_thread = find_any_thread_of_pid (pid);
-
-  the_low_target.arch_setup ();
-}
-
 /* Update all the target description of all processes; a new GDB
    connected, and it may or not support xml target descriptions.  */
 
@@ -873,7 +859,14 @@ x86_linux_update_xmltarget (void)
      release the current regcache objects.  */
   regcache_release ();
 
-  for_each_inferior (&all_processes, x86_arch_setup_process_callback);
+  for_each_process ([] (process_info *proc) {
+    int pid = proc->pid;
+
+    /* Look up any thread of this process.  */
+    current_thread = find_any_thread_of_pid (pid);
+
+    the_low_target.arch_setup ();
+  });
 
   current_thread = saved_thread;
 }
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 3d2c2ff5a0..14811c0390 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2615,8 +2615,7 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
       if (own_buf[sizeof ("qAttached") - 1])
 	{
 	  int pid = strtoul (own_buf + sizeof ("qAttached:") - 1, NULL, 16);
-	  process = (struct process_info *)
-	    find_inferior_id (&all_processes, pid_to_ptid (pid));
+	  process = find_process_pid (pid);
 	}
       else
 	{
@@ -3305,16 +3304,6 @@ gdb_wants_all_threads_stopped (void)
   for_each_inferior (&all_threads, gdb_wants_thread_stopped);
 }
 
-/* Clear the gdb_detached flag of every process.  */
-
-static void
-gdb_reattached_process (struct inferior_list_entry *entry)
-{
-  struct process_info *process = (struct process_info *) entry;
-
-  process->gdb_detached = 0;
-}
-
 /* Callback for for_each_inferior.  Clear the thread's pending status
    flag.  */
 
@@ -3363,7 +3352,9 @@ static void
 handle_status (char *own_buf)
 {
   /* GDB is connected, don't forward events to the target anymore.  */
-  for_each_inferior (&all_processes, gdb_reattached_process);
+  for_each_process ([] (process_info *process) {
+    process->gdb_detached = 0;
+  });
 
   /* In non-stop mode, we must send a stop reply for each stopped
      thread.  In all-stop mode, just send one for the first stopped
@@ -3517,64 +3508,14 @@ gdbserver_show_disableable (FILE *stream)
 }
 
 static void
-kill_inferior_callback (struct inferior_list_entry *entry)
+kill_inferior_callback (process_info *process)
 {
-  struct process_info *process = (struct process_info *) entry;
-  int pid = ptid_get_pid (process->entry.id);
+  int pid = process->pid;
 
   kill_inferior (pid);
   discard_queued_stop_replies (pid_to_ptid (pid));
 }
 
-/* Callback for for_each_inferior to detach or kill the inferior,
-   depending on whether we attached to it or not.
-   We inform the user whether we're detaching or killing the process
-   as this is only called when gdbserver is about to exit.  */
-
-static void
-detach_or_kill_inferior_callback (struct inferior_list_entry *entry)
-{
-  struct process_info *process = (struct process_info *) entry;
-  int pid = ptid_get_pid (process->entry.id);
-
-  if (process->attached)
-    detach_inferior (pid);
-  else
-    kill_inferior (pid);
-
-  discard_queued_stop_replies (pid_to_ptid (pid));
-}
-
-/* for_each_inferior callback for detach_or_kill_for_exit to print
-   the pids of started inferiors.  */
-
-static void
-print_started_pid (struct inferior_list_entry *entry)
-{
-  struct process_info *process = (struct process_info *) entry;
-
-  if (! process->attached)
-    {
-      int pid = ptid_get_pid (process->entry.id);
-      fprintf (stderr, " %d", pid);
-    }
-}
-
-/* for_each_inferior callback for detach_or_kill_for_exit to print
-   the pids of attached inferiors.  */
-
-static void
-print_attached_pid (struct inferior_list_entry *entry)
-{
-  struct process_info *process = (struct process_info *) entry;
-
-  if (process->attached)
-    {
-      int pid = ptid_get_pid (process->entry.id);
-      fprintf (stderr, " %d", pid);
-    }
-}
-
 /* Call this when exiting gdbserver with possible inferiors that need
    to be killed or detached from.  */
 
@@ -3589,19 +3530,37 @@ detach_or_kill_for_exit (void)
   if (have_started_inferiors_p ())
     {
       fprintf (stderr, "Killing process(es):");
-      for_each_inferior (&all_processes, print_started_pid);
+
+      for_each_process ([] (process_info *process) {
+	if (!process->attached)
+	  fprintf (stderr, " %d", process->pid);
+      });
+
       fprintf (stderr, "\n");
     }
   if (have_attached_inferiors_p ())
     {
       fprintf (stderr, "Detaching process(es):");
-      for_each_inferior (&all_processes, print_attached_pid);
+
+      for_each_process ([] (process_info *process) {
+	if (process->attached)
+	  fprintf (stderr, " %d", process->pid);
+      });
+
       fprintf (stderr, "\n");
     }
 
   /* Now we can kill or detach the inferiors.  */
+  for_each_process ([] (process_info *process) {
+    int pid = process->pid;
+
+    if (process->attached)
+      detach_inferior (pid);
+    else
+      kill_inferior (pid);
 
-  for_each_inferior (&all_processes, detach_or_kill_inferior_callback);
+    discard_queued_stop_replies (pid_to_ptid (pid));
+  });
 }
 
 /* Value that will be passed to exit(3) when gdbserver exits.  */
@@ -4332,7 +4291,8 @@ process_serial_event (void)
 	return 0;
 
       fprintf (stderr, "Killing all inferiors\n");
-      for_each_inferior (&all_processes, kill_inferior_callback);
+
+      for_each_process (kill_inferior_callback);
 
       /* When using the extended protocol, we wait with no program
 	 running.  The traditional protocol will exit instead.  */
@@ -4370,8 +4330,8 @@ process_serial_event (void)
       if (extended_protocol)
 	{
 	  if (target_running ())
-	    for_each_inferior (&all_processes,
-			       kill_inferior_callback);
+	    for_each_process (kill_inferior_callback);
+
 	  fprintf (stderr, "GDBserver restarting\n");
 
 	  /* Wait till we are at 1st instruction in prog.  */
-- 
2.14.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/3] gdbserver: use std::list for all_threads
  2017-10-09 14:30 [PATCH 0/3] Get rid of inferior_list Simon Marchi
  2017-10-09 14:30 ` [PATCH 2/3] gdbserver: Use std::list for all_processes Simon Marchi
@ 2017-10-09 14:30 ` Simon Marchi
  2017-10-09 14:30 ` [PATCH 1/3] gdbserver: Use std::list for all_dlls Simon Marchi
  2017-10-14 13:12 ` [PATCH 0/3] Get rid of inferior_list Simon Marchi
  3 siblings, 0 replies; 10+ messages in thread
From: Simon Marchi @ 2017-10-09 14:30 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

Remove the usage of inferior_list for the all_threads list in
gdbserver.  The entry field in thread_info is removed, and replaced by a
simple ptid field.

I added some functions to iterate (for_each_thread) and find threads
(find_thread).  However, changing all the users of find_inferior & co to
use these new functions would have made the patch way too big.  So I
opted instead to make find_inferior & co some shims, so that the
existing code only needs to be updated minimally.  We can then update
the existing code to use the new functions incrementally (I've started
to do the work, but I'll post it afterwards, see [1] if you want a peek).

This patch has been built-tested on all relevant platforms, except
lynx.  I also regtested using the native-gdbserver and
native-extended-gdbserver boards on x86.

[1] https://github.com/simark/binutils-gdb/commits/kill-inferior-list-entry

gdb/gdbserver/ChangeLog:

	* inferiors.h: (struct inferior_list): Remove.
	(struct inferior_list_entry); Remove.
	(add_inferior_to_list, clear_inferior_list, one_inferior_p,
	A_I_NEXT, ALL_INFERIORS_TYPE, ALL_INFERIORS, remove_inferior,
	get_first_inferior): Remove.
	(for_each_inferior, for_each_inferior_with_data, find_inferior,
	find_inferior_id, find_inferior_in_random): Change signature.
	* inferiors.c (all_threads): Change type to
	std::list<thread_info *>.
	(get_thread): Remove macro.
	(find_inferior, find_inferior_id): Change signature, implement
	using find_thread.
	(find_inferior_in_random): Change signature, implement using
	find_thread_in_random.
	(for_each_inferior, for_each_inferior_with_data): Change
	signature, implement using for_each_thread.
	(add_inferior_to_list, remove_inferior): Remove.
	(add_thread, get_first_thread, thread_of_pid,
	find_any_thread_of_pid, free_one_thread, remove_thread): Update.
	(get_first_inferior, one_inferior_p, clear_inferior_list):
	Remove.
	(clear_inferiors, get_thread_process): Update.
	* gdbthread.h: Include <list>.
	(struct thread_info) <entry>: Remove field.
	<id>: New field.
	(all_threads): Change type to std::list<thread_info *>.
	(get_first_inferior): Add doc.
	(find_thread, for_each_thread, find_thread_in_random): New
	functions.
	(current_ptid, pid_of, ptid_of, lwpid_of): Update.
	* linux-arm-low.c (update_registers_callback): Update.
	* linux-low.c (second_thread_of_pid_p): Update.
	(kill_one_lwp_callback, linux_detach_lwp_callback,
	delete_lwp_callback, status_pending_p_callback, same_lwp,
	find_lwp_pid, num_lwps, iterate_over_lwps_filter,
	iterate_over_lwps, not_stopped_callback,
	resume_stopped_resumed_lwps, count_events_callback,
	select_singlestep_lwp_callback, select_event_lwp_callback,
	unsuspend_one_lwp, linux_wait_1, send_sigstop_callback,
	suspend_and_send_sigstop_callback, wait_for_sigstop,
	stuck_in_jump_pad_callback, move_out_of_jump_pad_callback,
	lwp_running, linux_set_resume_request, resume_status_pending_p,
	need_step_over_p, start_step_over, linux_resume_one_thread,
	proceed_one_lwp, unsuspend_and_proceed_one_lwp,
	reset_lwp_ptrace_options_callback): Update.
	* linux-mips-low.c (update_watch_registers_callback): Update.
	* regcache.c (regcache_invalidate_one, regcache_invalidate):
	Update.
	(free_register_cache_thread_one): Remove.
	(regcache_release): Update.
	* server.c (handle_btrace_enable_bts, handle_btrace_enable_pt,
	handle_qxfer_threads_worker): Update.
	(handle_query): Update, use list iterator.
	(visit_actioned_threads, handle_pending_status,
	queue_stop_reply_callback, gdb_wants_all_threads_stopped,
	clear_pending_status_callback, set_pending_status_callback,
	find_status_pending_thread_callback, handle_status,
	process_serial_event): Update.
	* target.c (thread_search_callback): Update.
	* thread-db.c (thread_db_get_tls_address): Update.
	* tracepoint.c (tracepoint_finished_step, tracepoint_was_hit):
	Update.
	* win32-i386-low.c (update_debug_registers_callback): Update.
	* win32-low.c (delete_thread_info, child_delete_thread,
	continue_one_thread, suspend_one_thread,
	get_child_debug_event): Adjust.
---
 gdb/gdbserver/gdbthread.h      |  91 +++++++++++++--
 gdb/gdbserver/inferiors.c      | 244 ++++++++++-------------------------------
 gdb/gdbserver/inferiors.h      |  74 +++----------
 gdb/gdbserver/linux-arm-low.c  |   3 +-
 gdb/gdbserver/linux-low.c      | 172 ++++++++++++-----------------
 gdb/gdbserver/linux-mips-low.c |   4 +-
 gdb/gdbserver/lynx-low.c       |  10 +-
 gdb/gdbserver/regcache.c       |  18 +--
 gdb/gdbserver/server.c         |  75 ++++++-------
 gdb/gdbserver/target.c         |   7 +-
 gdb/gdbserver/thread-db.c      |   4 +-
 gdb/gdbserver/tracepoint.c     |   8 +-
 gdb/gdbserver/win32-i386-low.c |   4 +-
 gdb/gdbserver/win32-low.c      |  16 +--
 14 files changed, 276 insertions(+), 454 deletions(-)

diff --git a/gdb/gdbserver/gdbthread.h b/gdb/gdbserver/gdbthread.h
index 93688a36e4..8ace051036 100644
--- a/gdb/gdbserver/gdbthread.h
+++ b/gdb/gdbserver/gdbthread.h
@@ -22,14 +22,15 @@
 #include "common-gdbthread.h"
 #include "inferiors.h"
 
+#include <list>
+
 struct btrace_target_info;
 struct regcache;
 
 struct thread_info
 {
-  /* This must appear first.  See inferiors.h.
-     The list iterator functions assume it.  */
-  struct inferior_list_entry entry;
+  /* The id of this thread.  */
+  ptid_t id;
 
   void *target_data;
   struct regcache *regcache_data;
@@ -72,11 +73,13 @@ struct thread_info
   struct btrace_target_info *btrace;
 };
 
-extern struct inferior_list all_threads;
+extern std::list<thread_info *> all_threads;
 
 void remove_thread (struct thread_info *thread);
 struct thread_info *add_thread (ptid_t ptid, void *target_data);
 
+/* Return a pointer to the first thread, or NULL if there isn't one.  */
+
 struct thread_info *get_first_thread (void);
 
 struct thread_info *find_thread_ptid (ptid_t ptid);
@@ -85,15 +88,87 @@ struct thread_info *find_thread_ptid (ptid_t ptid);
    found.  */
 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;
+}
+
+/* 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;
+    }
+}
+
+/* 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 *thread) {
+    return func (thread) && (random_selector-- == 0);
+  });
+
+  gdb_assert (thread != NULL);
+
+  return thread;
+}
+
 /* Get current thread ID (Linux task ID).  */
-#define current_ptid (current_thread->entry.id)
+#define current_ptid (current_thread->id)
 
 /* Get the ptid of THREAD.  */
 
 static inline ptid_t
 ptid_of (const thread_info *thread)
 {
-  return thread->entry.id;
+  return thread->id;
 }
 
 /* Get the pid of THREAD.  */
@@ -101,7 +176,7 @@ ptid_of (const thread_info *thread)
 static inline int
 pid_of (const thread_info *thread)
 {
-  return thread->entry.id.pid ();
+  return thread->id.pid ();
 }
 
 /* Get the lwp of THREAD.  */
@@ -109,7 +184,7 @@ pid_of (const thread_info *thread)
 static inline long
 lwpid_of (const thread_info *thread)
 {
-  return thread->entry.id.lwp ();
+  return thread->id.lwp ();
 }
 
 /* Create a cleanup to restore current_thread.  */
diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
index 13ee8c97eb..564121fdd9 100644
--- a/gdb/gdbserver/inferiors.c
+++ b/gdb/gdbserver/inferiors.c
@@ -23,86 +23,68 @@
 #include "dll.h"
 
 std::list<process_info *> all_processes;
-struct inferior_list all_threads;
+std::list<thread_info *> all_threads;
 
 struct thread_info *current_thread;
 
-#define get_thread(inf) ((struct thread_info *)(inf))
-
 /* The current working directory used to start the inferior.  */
 static const char *current_inferior_cwd = NULL;
 
-void
-add_inferior_to_list (struct inferior_list *list,
-		      struct inferior_list_entry *new_inferior)
+thread_info *
+find_inferior (std::list<thread_info *> *thread_list,
+	       int (*func) (thread_info *, void *),
+	       void *arg)
 {
-  new_inferior->next = NULL;
-  if (list->tail != NULL)
-    list->tail->next = new_inferior;
-  else
-    list->head = new_inferior;
-  list->tail = new_inferior;
-}
+  gdb_assert (thread_list == &all_threads);
 
-/* Invoke ACTION for each inferior in LIST.  */
+  return find_thread ([&] (thread_info *thread) {
+    return func (thread, arg);
+  });
+}
 
-void
-for_each_inferior (struct inferior_list *list,
-		   void (*action) (struct inferior_list_entry *))
+thread_info *
+find_inferior_id (std::list<thread_info *> *thread_list, ptid_t id)
 {
-  struct inferior_list_entry *cur = list->head, *next;
-
-  while (cur != NULL)
-    {
-      next = cur->next;
-      (*action) (cur);
-      cur = next;
-    }
-}
+  gdb_assert (thread_list == &all_threads);
 
-/* Invoke ACTION for each inferior in LIST, passing DATA to ACTION.  */
+  return find_thread ([&] (thread_info *thread) {
+    return thread->id == id;
+  });
+}
 
-void
-for_each_inferior_with_data (struct inferior_list *list,
-			     void (*action) (struct inferior_list_entry *,
-					     void *),
-			     void *data)
+thread_info *
+find_inferior_in_random (std::list<thread_info *> *thread_list,
+			 int (*func) (thread_info *, void *),
+			 void *arg)
 {
-  struct inferior_list_entry *cur = list->head, *next;
-
-  while (cur != NULL)
-    {
-      next = cur->next;
-      (*action) (cur, data);
-      cur = next;
-    }
+  gdb_assert (thread_list == &all_threads);
+
+  return find_thread_in_random ([&] (thread_info *thread) {
+    return func (thread, arg);
+  });
 }
 
 void
-remove_inferior (struct inferior_list *list,
-		 struct inferior_list_entry *entry)
+for_each_inferior (std::list<thread_info *> *thread_list,
+		   void (*action) (thread_info *))
 {
-  struct inferior_list_entry **cur;
-
-  if (list->head == entry)
-    {
-      list->head = entry->next;
-      if (list->tail == entry)
-	list->tail = list->head;
-      return;
-    }
+  gdb_assert (thread_list == &all_threads);
 
-  cur = &list->head;
-  while (*cur && (*cur)->next != entry)
-    cur = &(*cur)->next;
-
-  if (*cur == NULL)
-    return;
+  for_each_thread ([&] (thread_info *thread) {
+    action (thread);
+  });
+}
 
-  (*cur)->next = entry->next;
+void
+for_each_inferior_with_data (std::list<thread_info *> *thread_list,
+			     void (*action) (thread_info *, void *),
+			     void *data)
+{
+  gdb_assert (thread_list == &all_threads);
 
-  if (list->tail == entry)
-    list->tail = *cur;
+  for_each_thread ([&] (thread_info *thread) {
+    action (thread, data);
+  });
 }
 
 struct thread_info *
@@ -110,11 +92,11 @@ add_thread (ptid_t thread_id, void *target_data)
 {
   struct thread_info *new_thread = XCNEW (struct thread_info);
 
-  new_thread->entry.id = thread_id;
+  new_thread->id = thread_id;
   new_thread->last_resume_kind = resume_continue;
   new_thread->last_status.kind = TARGET_WAITKIND_IGNORE;
 
-  add_inferior_to_list (&all_threads, &new_thread->entry);
+  all_threads.push_back (new_thread);
 
   if (current_thread == NULL)
     current_thread = new_thread;
@@ -124,12 +106,15 @@ add_thread (ptid_t thread_id, void *target_data)
   return new_thread;
 }
 
-/* Wrapper around get_first_inferior to return a struct thread_info *.  */
+/* See gdbthread.h.  */
 
 struct thread_info *
 get_first_thread (void)
 {
-  return (struct thread_info *) get_first_inferior (&all_threads);
+  if (!all_threads.empty ())
+    return all_threads.front ();
+  else
+    return NULL;
 }
 
 struct thread_info *
@@ -151,7 +136,7 @@ find_thread_process (const struct process_info *const process)
    matches a PID.  */
 
 static int
-thread_of_pid (struct inferior_list_entry *entry, void *pid_p)
+thread_of_pid (thread_info *entry, void *pid_p)
 {
   int pid = *(int *) pid_p;
 
@@ -163,17 +148,12 @@ thread_of_pid (struct inferior_list_entry *entry, void *pid_p)
 struct thread_info *
 find_any_thread_of_pid (int pid)
 {
-  struct inferior_list_entry *entry;
-
-  entry = find_inferior (&all_threads, thread_of_pid, &pid);
-
-  return (struct thread_info *) entry;
+  return find_inferior (&all_threads, thread_of_pid, &pid);
 }
 
 static void
-free_one_thread (struct inferior_list_entry *inf)
+free_one_thread (thread_info *thread)
 {
-  struct thread_info *thread = get_thread (inf);
   free_register_cache (thread_regcache_data (thread));
   free (thread);
 }
@@ -185,106 +165,12 @@ remove_thread (struct thread_info *thread)
     target_disable_btrace (thread->btrace);
 
   discard_queued_stop_replies (ptid_of (thread));
-  remove_inferior (&all_threads, (struct inferior_list_entry *) thread);
-  free_one_thread (&thread->entry);
+  all_threads.remove (thread);
+  free_one_thread (thread);
   if (current_thread == thread)
     current_thread = NULL;
 }
 
-/* Return a pointer to the first inferior in LIST, or NULL if there isn't one.
-   This is for cases where the caller needs a thread, but doesn't care
-   which one.  */
-
-struct inferior_list_entry *
-get_first_inferior (struct inferior_list *list)
-{
-  if (list->head != NULL)
-    return list->head;
-  return NULL;
-}
-
-/* Find the first inferior_list_entry E in LIST for which FUNC (E, ARG)
-   returns non-zero.  If no entry is found then return NULL.  */
-
-struct inferior_list_entry *
-find_inferior (struct inferior_list *list,
-	       int (*func) (struct inferior_list_entry *, void *), void *arg)
-{
-  struct inferior_list_entry *inf = list->head;
-
-  while (inf != NULL)
-    {
-      struct inferior_list_entry *next;
-
-      next = inf->next;
-      if ((*func) (inf, arg))
-	return inf;
-      inf = next;
-    }
-
-  return NULL;
-}
-
-/* Find the random inferior_list_entry E in LIST for which FUNC (E, ARG)
-   returns non-zero.  If no entry is found then return NULL.  */
-
-struct inferior_list_entry *
-find_inferior_in_random (struct inferior_list *list,
-			 int (*func) (struct inferior_list_entry *, void *),
-			 void *arg)
-{
-  struct inferior_list_entry *inf = list->head;
-  int count = 0;
-  int random_selector;
-
-  /* First count how many interesting entries we have.  */
-  while (inf != NULL)
-    {
-      struct inferior_list_entry *next;
-
-      next = inf->next;
-      if ((*func) (inf, arg))
-	count++;
-      inf = next;
-    }
-
-  if (count == 0)
-    return NULL;
-
-  /* Now randomly pick an entry out of those.  */
-  random_selector = (int)
-    ((count * (double) rand ()) / (RAND_MAX + 1.0));
-
-  inf = list->head;
-  while (inf != NULL)
-    {
-      struct inferior_list_entry *next;
-
-      next = inf->next;
-      if ((*func) (inf, arg) && (random_selector-- == 0))
-	return inf;
-      inf = next;
-    }
-
-  gdb_assert_not_reached ("failed to find an inferior in random.");
-  return NULL;
-}
-
-struct inferior_list_entry *
-find_inferior_id (struct inferior_list *list, ptid_t id)
-{
-  struct inferior_list_entry *inf = list->head;
-
-  while (inf != NULL)
-    {
-      if (ptid_equal (inf->id, id))
-	return inf;
-      inf = inf->next;
-    }
-
-  return NULL;
-}
-
 void *
 thread_target_data (struct thread_info *thread)
 {
@@ -303,28 +189,11 @@ set_thread_regcache_data (struct thread_info *thread, struct regcache *data)
   thread->regcache_data = data;
 }
 
-/* Return true if LIST has exactly one entry.  */
-
-int
-one_inferior_p (struct inferior_list *list)
-{
-  return list->head != NULL && list->head == list->tail;
-}
-
-/* Reset head,tail of LIST, assuming all entries have already been freed.  */
-
-void
-clear_inferior_list (struct inferior_list *list)
-{
-  list->head = NULL;
-  list->tail = NULL;
-}
-
 void
 clear_inferiors (void)
 {
   for_each_inferior (&all_threads, free_one_thread);
-  clear_inferior_list (&all_threads);
+  all_threads.clear ();
 
   clear_dlls ();
 
@@ -402,8 +271,7 @@ have_attached_inferiors_p (void)
 struct process_info *
 get_thread_process (const struct thread_info *thread)
 {
-  int pid = ptid_get_pid (thread->entry.id);
-  return find_process_pid (pid);
+  return find_process_pid (thread->id.pid ());
 }
 
 struct process_info *
diff --git a/gdb/gdbserver/inferiors.h b/gdb/gdbserver/inferiors.h
index 674b8c6dcb..fb0e2fda81 100644
--- a/gdb/gdbserver/inferiors.h
+++ b/gdb/gdbserver/inferiors.h
@@ -22,19 +22,6 @@
 #include "gdb_vecs.h"
 #include <list>
 
-/* Generic information for tracking a list of ``inferiors'' - threads,
-   processes, etc.  */
-struct inferior_list
-{
-  struct inferior_list_entry *head;
-  struct inferior_list_entry *tail;
-};
-struct inferior_list_entry
-{
-  ptid_t id;
-  struct inferior_list_entry *next;
-};
-
 struct thread_info;
 struct regcache;
 struct target_desc;
@@ -96,38 +83,6 @@ struct process_info *get_thread_process (const struct thread_info *);
 
 extern std::list<process_info *> all_processes;
 
-void add_inferior_to_list (struct inferior_list *list,
-			   struct inferior_list_entry *new_inferior);
-void for_each_inferior (struct inferior_list *list,
-			void (*action) (struct inferior_list_entry *));
-
-void for_each_inferior_with_data
-  (struct inferior_list *list,
-   void (*action) (struct inferior_list_entry *, void *),
-   void *data);
-
-void clear_inferior_list (struct inferior_list *list);
-
-int one_inferior_p (struct inferior_list *list);
-
-/* Helper for ALL_INFERIORS_TYPE.  Gets the next element starting at
-   CUR, if CUR is not NULL.  */
-#define A_I_NEXT(type, list, cur)					\
-  ((cur) != NULL							\
-   ? (type *) ((struct inferior_list_entry *) cur)->next		\
-   : NULL)
-
-/* Iterate over all inferiors of type TYPE in LIST, open loop
-   style.  */
-#define ALL_INFERIORS_TYPE(type, list, cur, tmp)				\
-  for ((cur) = (type *) (list)->head, (tmp) = A_I_NEXT (type, list, cur); \
-       (cur) != NULL;							\
-       (cur) = (tmp), (tmp) = A_I_NEXT (type, list, cur))
-
-/* Iterate over all inferiors in LIST, open loop style.  */
-#define ALL_INFERIORS(list, cur, tmp)				\
-  ALL_INFERIORS_TYPE (struct inferior_list_entry, list, cur, tmp)
-
 /* Invoke FUNC for each process.  */
 
 template <typename Func>
@@ -169,10 +124,6 @@ find_process (Func func)
 }
 
 extern struct thread_info *current_thread;
-void remove_inferior (struct inferior_list *list,
-		      struct inferior_list_entry *entry);
-
-struct inferior_list_entry *get_first_inferior (struct inferior_list *list);
 
 /* Return the first process in the processes list.  */
 struct process_info *get_first_process (void);
@@ -184,18 +135,19 @@ int have_started_inferiors_p (void);
 int have_attached_inferiors_p (void);
 
 void clear_inferiors (void);
-struct inferior_list_entry *find_inferior
-     (struct inferior_list *,
-      int (*func) (struct inferior_list_entry *,
-		   void *),
-      void *arg);
-struct inferior_list_entry *find_inferior_id (struct inferior_list *list,
-					      ptid_t id);
-struct inferior_list_entry *
-  find_inferior_in_random (struct inferior_list *,
-			   int (*func) (struct inferior_list_entry *,
-					void *),
-			   void *arg);
+
+thread_info *find_inferior (std::list<thread_info *> *thread_list,
+			    int (*func) (thread_info *, void *), void *arg);
+thread_info *find_inferior_id (std::list<thread_info *> *thread_list,
+			       ptid_t id);
+thread_info *find_inferior_in_random (std::list<thread_info *> *thread_list,
+				      int (*func) (thread_info *, void *),
+				      void *arg);
+void for_each_inferior (std::list<thread_info *> *thread_list,
+			void (*action) (thread_info *));
+void for_each_inferior_with_data (std::list<thread_info *> *thread_list,
+				  void (*action) (thread_info *, void *),
+				  void *data);
 
 void *thread_target_data (struct thread_info *);
 struct regcache *thread_regcache_data (struct thread_info *);
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index cb59c2a130..c60661dc33 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -467,9 +467,8 @@ struct update_registers_data
 };
 
 static int
-update_registers_callback (struct inferior_list_entry *entry, void *arg)
+update_registers_callback (thread_info *thread, void *arg)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
   struct update_registers_data *data = (struct update_registers_data *) arg;
 
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index dc0ad85734..58dcec1cb9 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -279,7 +279,7 @@ static void enqueue_pending_signal (struct lwp_info *lwp, int signal, siginfo_t
 static void complete_ongoing_step_over (void);
 static int linux_low_ptrace_options (int attached);
 static int check_ptrace_stopped_lwp_gone (struct lwp_info *lp);
-static int proceed_one_lwp (struct inferior_list_entry *entry, void *except);
+static int proceed_one_lwp (thread_info *thread, void *except);
 
 /* When the event-loop is doing a step-over, this points at the thread
    being stepped.  */
@@ -1243,11 +1243,11 @@ struct counter
 };
 
 static int
-second_thread_of_pid_p (struct inferior_list_entry *entry, void *args)
+second_thread_of_pid_p (thread_info *thread, void *args)
 {
   struct counter *counter = (struct counter *) args;
 
-  if (ptid_get_pid (entry->id) == counter->pid)
+  if (thread->id.pid () == counter->pid)
     {
       if (++counter->count > 1)
 	return 1;
@@ -1356,13 +1356,12 @@ kill_wait_lwp (struct lwp_info *lwp)
    except the leader.  */
 
 static int
-kill_one_lwp_callback (struct inferior_list_entry *entry, void *args)
+kill_one_lwp_callback (thread_info *thread, void *args)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
   int pid = * (int *) args;
 
-  if (ptid_get_pid (entry->id) != pid)
+  if (thread->id.pid () != pid)
     return 0;
 
   /* We avoid killing the first thread here, because of a Linux kernel (at
@@ -1374,7 +1373,7 @@ kill_one_lwp_callback (struct inferior_list_entry *entry, void *args)
     {
       if (debug_threads)
 	debug_printf ("lkop: is last of process %s\n",
-		      target_pid_to_str (entry->id));
+		      target_pid_to_str (thread->id));
       return 0;
     }
 
@@ -1589,21 +1588,20 @@ linux_detach_one_lwp (struct lwp_info *lwp)
    given process.  */
 
 static int
-linux_detach_lwp_callback (struct inferior_list_entry *entry, void *args)
+linux_detach_lwp_callback (thread_info *thread, void *args)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
   int pid = *(int *) args;
   int lwpid = lwpid_of (thread);
 
   /* Skip other processes.  */
-  if (ptid_get_pid (entry->id) != pid)
+  if (thread->id.pid () != pid)
     return 0;
 
   /* We don't actually detach from the thread group leader just yet.
      If the thread group exits, we must reap the zombie clone lwps
      before we're able to reap the leader.  */
-  if (ptid_get_pid (entry->id) == lwpid)
+  if (thread->id.pid () == lwpid)
     return 0;
 
   linux_detach_one_lwp (lwp);
@@ -1657,9 +1655,8 @@ linux_detach (int pid)
 /* Remove all LWPs that belong to process PROC from the lwp list.  */
 
 static int
-delete_lwp_callback (struct inferior_list_entry *entry, void *proc)
+delete_lwp_callback (thread_info *thread, void *proc)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
   struct process_info *process = (struct process_info *) proc;
 
@@ -1806,9 +1803,8 @@ lwp_resumed (struct lwp_info *lwp)
 
 /* Return 1 if this lwp has an interesting status pending.  */
 static int
-status_pending_p_callback (struct inferior_list_entry *entry, void *arg)
+status_pending_p_callback (thread_info *thread, void *arg)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lp = get_thread_lwp (thread);
   ptid_t ptid = * (ptid_t *) arg;
 
@@ -1831,7 +1827,7 @@ status_pending_p_callback (struct inferior_list_entry *entry, void *arg)
 }
 
 static int
-same_lwp (struct inferior_list_entry *entry, void *data)
+same_lwp (thread_info *thread, void *data)
 {
   ptid_t ptid = *(ptid_t *) data;
   int lwp;
@@ -1841,7 +1837,7 @@ same_lwp (struct inferior_list_entry *entry, void *data)
   else
     lwp = ptid_get_pid (ptid);
 
-  if (ptid_get_lwp (entry->id) == lwp)
+  if (thread->id.lwp () == lwp)
     return 1;
 
   return 0;
@@ -1850,13 +1846,12 @@ same_lwp (struct inferior_list_entry *entry, void *data)
 struct lwp_info *
 find_lwp_pid (ptid_t ptid)
 {
-  struct inferior_list_entry *thread
-    = find_inferior (&all_threads, same_lwp, &ptid);
+  thread_info *thread = find_inferior (&all_threads, same_lwp, &ptid);
 
   if (thread == NULL)
     return NULL;
 
-  return get_thread_lwp ((struct thread_info *) thread);
+  return get_thread_lwp (thread);
 }
 
 /* Return the number of known LWPs in the tgid given by PID.  */
@@ -1864,14 +1859,12 @@ find_lwp_pid (ptid_t ptid)
 static int
 num_lwps (int pid)
 {
-  struct inferior_list_entry *inf, *tmp;
   int count = 0;
 
-  ALL_INFERIORS (&all_threads, inf, tmp)
-    {
-      if (ptid_get_pid (inf->id) == pid)
-	count++;
-    }
+  for_each_thread ([&] (thread_info *thread) {
+    if (thread->id.pid () == pid)
+      count++;
+  });
 
   return count;
 }
@@ -1897,15 +1890,14 @@ struct iterate_over_lwps_args
    find_inferiors should continue iterating.  */
 
 static int
-iterate_over_lwps_filter (struct inferior_list_entry *entry, void *args_p)
+iterate_over_lwps_filter (thread_info *thread, void *args_p)
 {
   struct iterate_over_lwps_args *args
     = (struct iterate_over_lwps_args *) args_p;
 
-  if (ptid_match (entry->id, args->filter))
+  if (thread->id.matches (args->filter))
     {
-      struct thread_info *thr = (struct thread_info *) entry;
-      struct lwp_info *lwp = get_thread_lwp (thr);
+      struct lwp_info *lwp = get_thread_lwp (thread);
 
       return (*args->callback) (lwp, args->data);
     }
@@ -1921,13 +1913,13 @@ iterate_over_lwps (ptid_t filter,
 		   void *data)
 {
   struct iterate_over_lwps_args args = {filter, callback, data};
-  struct inferior_list_entry *entry;
 
-  entry = find_inferior (&all_threads, iterate_over_lwps_filter, &args);
-  if (entry == NULL)
+  thread_info *thread = find_inferior (&all_threads, iterate_over_lwps_filter,
+				       &args);
+  if (thread == NULL)
     return NULL;
 
-  return get_thread_lwp ((struct thread_info *) entry);
+  return get_thread_lwp (thread);
 }
 
 /* Detect zombie thread group leaders, and "exit" them.  We can't reap
@@ -1996,16 +1988,15 @@ check_zombie_leaders (void)
    stopped.  ARG is a PTID filter.  */
 
 static int
-not_stopped_callback (struct inferior_list_entry *entry, void *arg)
+not_stopped_callback (thread_info *thread, void *arg)
 {
-  struct thread_info *thr = (struct thread_info *) entry;
   struct lwp_info *lwp;
   ptid_t filter = *(ptid_t *) arg;
 
-  if (!ptid_match (ptid_of (thr), filter))
+  if (!ptid_match (ptid_of (thread), filter))
     return 0;
 
-  lwp = get_thread_lwp (thr);
+  lwp = get_thread_lwp (thread);
   if (!lwp->stopped)
     return 1;
 
@@ -2651,9 +2642,8 @@ maybe_hw_step (struct thread_info *thread)
    to report, but are resumed from the core's perspective.  */
 
 static void
-resume_stopped_resumed_lwps (struct inferior_list_entry *entry)
+resume_stopped_resumed_lwps (thread_info *thread)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lp = get_thread_lwp (thread);
 
   if (lp->stopped
@@ -2893,9 +2883,8 @@ linux_wait_for_event (ptid_t ptid, int *wstatp, int options)
 /* Count the LWP's that have had events.  */
 
 static int
-count_events_callback (struct inferior_list_entry *entry, void *data)
+count_events_callback (thread_info *thread, void *data)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lp = get_thread_lwp (thread);
   int *count = (int *) data;
 
@@ -2912,9 +2901,8 @@ count_events_callback (struct inferior_list_entry *entry, void *data)
 /* Select the LWP (if any) that is currently being single-stepped.  */
 
 static int
-select_singlestep_lwp_callback (struct inferior_list_entry *entry, void *data)
+select_singlestep_lwp_callback (thread_info *thread, void *data)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lp = get_thread_lwp (thread);
 
   if (thread->last_status.kind == TARGET_WAITKIND_IGNORE
@@ -2928,9 +2916,8 @@ select_singlestep_lwp_callback (struct inferior_list_entry *entry, void *data)
 /* Select the Nth LWP that has had an event.  */
 
 static int
-select_event_lwp_callback (struct inferior_list_entry *entry, void *data)
+select_event_lwp_callback (thread_info *thread, void *data)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lp = get_thread_lwp (thread);
   int *selector = (int *) data;
 
@@ -3011,9 +2998,8 @@ select_event_lwp (struct lwp_info **orig_lp)
 /* Decrement the suspend count of an LWP.  */
 
 static int
-unsuspend_one_lwp (struct inferior_list_entry *entry, void *except)
+unsuspend_one_lwp (thread_info *thread, void *except)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
 
   /* Ignore EXCEPT.  */
@@ -3033,10 +3019,9 @@ unsuspend_all_lwps (struct lwp_info *except)
   find_inferior (&all_threads, unsuspend_one_lwp, except);
 }
 
-static void move_out_of_jump_pad_callback (struct inferior_list_entry *entry);
-static int stuck_in_jump_pad_callback (struct inferior_list_entry *entry,
-				       void *data);
-static int lwp_running (struct inferior_list_entry *entry, void *data);
+static void move_out_of_jump_pad_callback (thread_info *thread);
+static int stuck_in_jump_pad_callback (thread_info *thread, void *data);
+static int lwp_running (thread_info *thread, void *data);
 static ptid_t linux_wait_1 (ptid_t ptid,
 			    struct target_waitstatus *ourstatus,
 			    int target_options);
@@ -3772,18 +3757,16 @@ linux_wait_1 (ptid_t ptid,
 	{
 	  /* In all-stop, a stop reply cancels all previous resume
 	     requests.  Delete all single-step breakpoints.  */
-	  struct inferior_list_entry *inf, *tmp;
 
-	  ALL_INFERIORS (&all_threads, inf, tmp)
-	    {
-	      struct thread_info *thread = (struct thread_info *) inf;
+	  find_thread ([&] (thread_info *thread) {
+	    if (has_single_step_breakpoints (thread))
+	      {
+		remove_single_step_breakpoints_p = 1;
+		return true;
+	      }
 
-	      if (has_single_step_breakpoints (thread))
-		{
-		  remove_single_step_breakpoints_p = 1;
-		  break;
-		}
-	    }
+	    return false;
+	  });
 	}
 
       if (remove_single_step_breakpoints_p)
@@ -3800,15 +3783,10 @@ linux_wait_1 (ptid_t ptid,
 	    }
 	  else
 	    {
-	      struct inferior_list_entry *inf, *tmp;
-
-	      ALL_INFERIORS (&all_threads, inf, tmp)
-		{
-		  struct thread_info *thread = (struct thread_info *) inf;
-
-		  if (has_single_step_breakpoints (thread))
-		    delete_single_step_breakpoints (thread);
-		}
+	      for_each_thread ([] (thread_info *thread){
+		if (has_single_step_breakpoints (thread))
+		  delete_single_step_breakpoints (thread);
+	      });
 	    }
 
 	  unstop_all_lwps (0, event_child);
@@ -4061,9 +4039,8 @@ send_sigstop (struct lwp_info *lwp)
 }
 
 static int
-send_sigstop_callback (struct inferior_list_entry *entry, void *except)
+send_sigstop_callback (thread_info *thread, void *except)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
 
   /* Ignore EXCEPT.  */
@@ -4080,10 +4057,8 @@ send_sigstop_callback (struct inferior_list_entry *entry, void *except)
 /* Increment the suspend count of an LWP, and stop it, if not stopped
    yet.  */
 static int
-suspend_and_send_sigstop_callback (struct inferior_list_entry *entry,
-				   void *except)
+suspend_and_send_sigstop_callback (thread_info *thread, void *except)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
 
   /* Ignore EXCEPT.  */
@@ -4092,7 +4067,7 @@ suspend_and_send_sigstop_callback (struct inferior_list_entry *entry,
 
   lwp_suspended_inc (lwp);
 
-  return send_sigstop_callback (entry, except);
+  return send_sigstop_callback (thread, except);
 }
 
 static void
@@ -4145,7 +4120,7 @@ wait_for_sigstop (void)
 
   saved_thread = current_thread;
   if (saved_thread != NULL)
-    saved_tid = saved_thread->entry.id;
+    saved_tid = saved_thread->id;
   else
     saved_tid = null_ptid; /* avoid bogus unused warning */
 
@@ -4179,9 +4154,8 @@ wait_for_sigstop (void)
    because she wants to debug it.  */
 
 static int
-stuck_in_jump_pad_callback (struct inferior_list_entry *entry, void *data)
+stuck_in_jump_pad_callback (thread_info *thread, void *data)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
 
   if (lwp->suspended != 0)
@@ -4203,9 +4177,8 @@ stuck_in_jump_pad_callback (struct inferior_list_entry *entry, void *data)
 }
 
 static void
-move_out_of_jump_pad_callback (struct inferior_list_entry *entry)
+move_out_of_jump_pad_callback (thread_info *thread)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct thread_info *saved_thread;
   struct lwp_info *lwp = get_thread_lwp (thread);
   int *wstat;
@@ -4254,9 +4227,8 @@ move_out_of_jump_pad_callback (struct inferior_list_entry *entry)
 }
 
 static int
-lwp_running (struct inferior_list_entry *entry, void *data)
+lwp_running (thread_info *thread, void *data)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
 
   if (lwp_is_marked_dead (lwp))
@@ -4657,9 +4629,8 @@ struct thread_resume_array
    suspension).  */
 
 static int
-linux_set_resume_request (struct inferior_list_entry *entry, void *arg)
+linux_set_resume_request (thread_info *thread, void *arg)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
   int ndx;
   struct thread_resume_array *r;
@@ -4670,7 +4641,7 @@ linux_set_resume_request (struct inferior_list_entry *entry, void *arg)
     {
       ptid_t ptid = r->resume[ndx].thread;
       if (ptid_equal (ptid, minus_one_ptid)
-	  || ptid_equal (ptid, entry->id)
+	  || ptid == thread->id
 	  /* Handle both 'pPID' and 'pPID.-1' as meaning 'all threads
 	     of PID'.  */
 	  || (ptid_get_pid (ptid) == pid_of (thread)
@@ -4727,7 +4698,7 @@ linux_set_resume_request (struct inferior_list_entry *entry, void *arg)
 	     reported to GDBserver core, but GDB has not pulled the
 	     event out of the vStopped queue yet, likewise, ignore the
 	     (wildcard) resume request.  */
-	  if (in_queued_stop_replies (entry->id))
+	  if (in_queued_stop_replies (thread->id))
 	    {
 	      if (debug_threads)
 		debug_printf ("not resuming LWP %ld: has queued stop reply\n",
@@ -4771,9 +4742,8 @@ linux_set_resume_request (struct inferior_list_entry *entry, void *arg)
    Set *FLAG_P if this lwp has an interesting status pending.  */
 
 static int
-resume_status_pending_p (struct inferior_list_entry *entry, void *flag_p)
+resume_status_pending_p (thread_info *thread, void *flag_p)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
 
   /* LWPs which will not be resumed are not interesting, because
@@ -4793,9 +4763,8 @@ resume_status_pending_p (struct inferior_list_entry *entry, void *flag_p)
    inferior's regcache.  */
 
 static int
-need_step_over_p (struct inferior_list_entry *entry, void *dummy)
+need_step_over_p (thread_info *thread, void *dummy)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
   struct thread_info *saved_thread;
   CORE_ADDR pc;
@@ -4978,7 +4947,7 @@ start_step_over (struct lwp_info *lwp)
   linux_resume_one_lwp (lwp, step, 0, NULL);
 
   /* Require next event from this LWP.  */
-  step_over_bkpt = thread->entry.id;
+  step_over_bkpt = thread->id;
   return 1;
 }
 
@@ -5071,9 +5040,8 @@ complete_ongoing_step_over (void)
    they should be re-issued if necessary.  */
 
 static int
-linux_resume_one_thread (struct inferior_list_entry *entry, void *arg)
+linux_resume_one_thread (thread_info *thread, void *arg)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
   int leave_all_stopped = * (int *) arg;
   int leave_pending;
@@ -5164,7 +5132,7 @@ linux_resume_one_thread (struct inferior_list_entry *entry, void *arg)
       if (debug_threads)
 	debug_printf ("resuming LWP %ld\n", lwpid_of (thread));
 
-      proceed_one_lwp (entry, NULL);
+      proceed_one_lwp (thread, NULL);
     }
   else
     {
@@ -5256,9 +5224,8 @@ linux_resume (struct thread_resume *resume_info, size_t n)
    on that particular thread, and leave all others stopped.  */
 
 static int
-proceed_one_lwp (struct inferior_list_entry *entry, void *except)
+proceed_one_lwp (thread_info *thread, void *except)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
   int step;
 
@@ -5355,9 +5322,8 @@ proceed_one_lwp (struct inferior_list_entry *entry, void *except)
 }
 
 static int
-unsuspend_and_proceed_one_lwp (struct inferior_list_entry *entry, void *except)
+unsuspend_and_proceed_one_lwp (thread_info *thread, void *except)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
 
   if (lwp == except)
@@ -5365,7 +5331,7 @@ unsuspend_and_proceed_one_lwp (struct inferior_list_entry *entry, void *except)
 
   lwp_suspended_decr (lwp);
 
-  return proceed_one_lwp (entry, except);
+  return proceed_one_lwp (thread, except);
 }
 
 /* When we finish a step-over, set threads running again.  If there's
@@ -6467,10 +6433,8 @@ linux_supports_exec_events (void)
    options for the specified lwp.  */
 
 static int
-reset_lwp_ptrace_options_callback (struct inferior_list_entry *entry,
-				   void *args)
+reset_lwp_ptrace_options_callback (thread_info *thread, void *args)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
 
   if (!lwp->stopped)
diff --git a/gdb/gdbserver/linux-mips-low.c b/gdb/gdbserver/linux-mips-low.c
index b4a83b0e21..cee20eda67 100644
--- a/gdb/gdbserver/linux-mips-low.c
+++ b/gdb/gdbserver/linux-mips-low.c
@@ -293,10 +293,8 @@ mips_breakpoint_at (CORE_ADDR where)
    if the lwp's process id is *PID_P.  */
 
 static int
-update_watch_registers_callback (struct inferior_list_entry *entry,
-				 void *pid_p)
+update_watch_registers_callback (thread_info *thread, void *pid_p)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
   int pid = *(int *) pid_p;
 
diff --git a/gdb/gdbserver/lynx-low.c b/gdb/gdbserver/lynx-low.c
index f074dd50b3..ca9adc446b 100644
--- a/gdb/gdbserver/lynx-low.c
+++ b/gdb/gdbserver/lynx-low.c
@@ -559,16 +559,12 @@ lynx_detach (int pid)
    all threads belonging to process PROC.  */
 
 static int
-lynx_delete_thread_callback (struct inferior_list_entry *entry, void *proc)
+lynx_delete_thread_callback (thread_info *thread, void *proc)
 {
   struct process_info *process = (struct process_info *) proc;
 
-  if (ptid_get_pid (entry->id) == pid_of (process))
-    {
-      struct thread_info *thr = find_thread_ptid (entry->id);
-
-      remove_thread (thr);
-    }
+  if (thread->id.pid () == pid_of (process))
+    remove_thread (thread);
 
   return 0;
 }
diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
index 75480912fb..e45e60e5ca 100644
--- a/gdb/gdbserver/regcache.c
+++ b/gdb/gdbserver/regcache.c
@@ -94,14 +94,12 @@ regcache_invalidate_thread (struct thread_info *thread)
 }
 
 static int
-regcache_invalidate_one (struct inferior_list_entry *entry,
-			 void *pid_p)
+regcache_invalidate_one (thread_info *thread, void *pid_p)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   int pid = *(int *) pid_p;
 
   /* Only invalidate the regcaches of threads of this process.  */
-  if (ptid_get_pid (entry->id) == pid)
+  if (thread->id.pid () == pid)
     regcache_invalidate_thread (thread);
 
   return 0;
@@ -121,7 +119,7 @@ void
 regcache_invalidate (void)
 {
   /* Only update the threads of the current process.  */
-  int pid = ptid_get_pid (current_thread->entry.id);
+  int pid = current_thread->id.pid ();
 
   regcache_invalidate_pid (pid);
 }
@@ -290,19 +288,11 @@ free_register_cache_thread (struct thread_info *thread)
     }
 }
 
-static void
-free_register_cache_thread_one (struct inferior_list_entry *entry)
-{
-  struct thread_info *thread = (struct thread_info *) entry;
-
-  free_register_cache_thread (thread);
-}
-
 void
 regcache_release (void)
 {
   /* Flush and release all pre-existing register caches.  */
-  for_each_inferior (&all_threads, free_register_cache_thread_one);
+  for_each_inferior (&all_threads, free_register_cache_thread);
 }
 #endif
 
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 14811c0390..f8ec8b4955 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -387,8 +387,7 @@ handle_btrace_enable_bts (struct thread_info *thread)
     return "E.Btrace already enabled.";
 
   current_btrace_conf.format = BTRACE_FORMAT_BTS;
-  thread->btrace = target_enable_btrace (thread->entry.id,
-					 &current_btrace_conf);
+  thread->btrace = target_enable_btrace (thread->id, &current_btrace_conf);
   if (thread->btrace == NULL)
     return "E.Could not enable btrace.";
 
@@ -404,8 +403,7 @@ handle_btrace_enable_pt (struct thread_info *thread)
     return "E.Btrace already enabled.";
 
   current_btrace_conf.format = BTRACE_FORMAT_PT;
-  thread->btrace = target_enable_btrace (thread->entry.id,
-					 &current_btrace_conf);
+  thread->btrace = target_enable_btrace (thread->id, &current_btrace_conf);
   if (thread->btrace == NULL)
     return "E.Could not enable btrace.";
 
@@ -1652,9 +1650,8 @@ handle_qxfer_statictrace (const char *annex,
    Emit the XML to describe the thread of INF.  */
 
 static void
-handle_qxfer_threads_worker (struct inferior_list_entry *inf, void *arg)
+handle_qxfer_threads_worker (thread_info *thread, void *arg)
 {
-  struct thread_info *thread = (struct thread_info *) inf;
   struct buffer *buffer = (struct buffer *) arg;
   ptid_t ptid = ptid_of (thread);
   char ptid_s[100];
@@ -2145,7 +2142,7 @@ supported_btrace_packets (char *buf)
 static void
 handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 {
-  static struct inferior_list_entry *thread_ptr;
+  static std::list<thread_info *>::const_iterator thread_iter;
 
   /* Reply the current thread id.  */
   if (strcmp ("qC", own_buf) == 0 && !disable_packet_qC)
@@ -2157,8 +2154,8 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 	ptid = general_thread;
       else
 	{
-	  thread_ptr = get_first_inferior (&all_threads);
-	  ptid = thread_ptr->id;
+	  thread_iter = all_threads.begin ();
+	  ptid = (*thread_iter)->id;
 	}
 
       sprintf (own_buf, "QC");
@@ -2220,22 +2217,24 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
       if (strcmp ("qfThreadInfo", own_buf) == 0)
 	{
 	  require_running_or_return (own_buf);
-	  thread_ptr = get_first_inferior (&all_threads);
+	  thread_iter = all_threads.begin ();
 
 	  *own_buf++ = 'm';
-	  write_ptid (own_buf, thread_ptr->id);
-	  thread_ptr = thread_ptr->next;
+	  ptid_t ptid = (*thread_iter)->id;
+	  write_ptid (own_buf, ptid);
+	  thread_iter++;
 	  return;
 	}
 
       if (strcmp ("qsThreadInfo", own_buf) == 0)
 	{
 	  require_running_or_return (own_buf);
-	  if (thread_ptr != NULL)
+	  if (thread_iter != all_threads.end ())
 	    {
 	      *own_buf++ = 'm';
-	      write_ptid (own_buf, thread_ptr->id);
-	      thread_ptr = thread_ptr->next;
+	      ptid_t ptid = (*thread_iter)->id;
+	      write_ptid (own_buf, ptid);
+	      thread_iter++;
 	      return;
 	    }
 	  else
@@ -2693,7 +2692,7 @@ struct visit_actioned_threads_data
    Note: This function is itself a callback for find_inferior.  */
 
 static int
-visit_actioned_threads (struct inferior_list_entry *entry, void *datap)
+visit_actioned_threads (thread_info *thread, void *datap)
 {
   struct visit_actioned_threads_data *data
     = (struct visit_actioned_threads_data *) datap;
@@ -2707,13 +2706,11 @@ visit_actioned_threads (struct inferior_list_entry *entry, void *datap)
       const struct thread_resume *action = &actions[i];
 
       if (ptid_equal (action->thread, minus_one_ptid)
-	  || ptid_equal (action->thread, entry->id)
+	  || ptid_equal (action->thread, thread->id)
 	  || ((ptid_get_pid (action->thread)
-	       == ptid_get_pid (entry->id))
+	       == thread->id.pid ())
 	      && ptid_get_lwp (action->thread) == -1))
 	{
-	  struct thread_info *thread = (struct thread_info *) entry;
-
 	  if ((*callback) (action, thread))
 	    return 1;
 	}
@@ -2734,7 +2731,7 @@ handle_pending_status (const struct thread_resume *resumption,
       thread->status_pending_p = 0;
 
       last_status = thread->last_status;
-      last_ptid = thread->entry.id;
+      last_ptid = thread->id;
       prepare_resume_reply (own_buf, last_ptid, &last_status);
       return 1;
     }
@@ -3234,17 +3231,15 @@ myresume (char *own_buf, int step, int sig)
    stopped thread.  */
 
 static int
-queue_stop_reply_callback (struct inferior_list_entry *entry, void *arg)
+queue_stop_reply_callback (thread_info *thread, void *arg)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
-
   /* For now, assume targets that don't have this callback also don't
      manage the thread's last_status field.  */
   if (the_target->thread_stopped == NULL)
     {
       struct vstop_notif *new_notif = XNEW (struct vstop_notif);
 
-      new_notif->ptid = entry->id;
+      new_notif->ptid = thread->id;
       new_notif->status = thread->last_status;
       /* Pass the last stop reply back to GDB, but don't notify
 	 yet.  */
@@ -3261,7 +3256,7 @@ queue_stop_reply_callback (struct inferior_list_entry *entry, void *arg)
 		= target_waitstatus_to_string (&thread->last_status);
 
 	      debug_printf ("Reporting thread %s as already stopped with %s\n",
-			    target_pid_to_str (entry->id),
+			    target_pid_to_str (thread->id),
 			    status_string.c_str ());
 	    }
 
@@ -3269,7 +3264,7 @@ queue_stop_reply_callback (struct inferior_list_entry *entry, void *arg)
 
 	  /* Pass the last stop reply back to GDB, but don't notify
 	     yet.  */
-	  queue_stop_reply (entry->id, &thread->last_status);
+	  queue_stop_reply (thread->id, &thread->last_status);
 	}
     }
 
@@ -3281,10 +3276,8 @@ queue_stop_reply_callback (struct inferior_list_entry *entry, void *arg)
    it.  */
 
 static void
-gdb_wants_thread_stopped (struct inferior_list_entry *entry)
+gdb_wants_thread_stopped (thread_info *thread)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
-
   thread->last_resume_kind = resume_stop;
 
   if (thread->last_status.kind == TARGET_WAITKIND_IGNORE)
@@ -3308,10 +3301,8 @@ gdb_wants_all_threads_stopped (void)
    flag.  */
 
 static void
-clear_pending_status_callback (struct inferior_list_entry *entry)
+clear_pending_status_callback (thread_info *thread)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
-
   thread->status_pending_p = 0;
 }
 
@@ -3319,10 +3310,8 @@ clear_pending_status_callback (struct inferior_list_entry *entry)
    interesting event, mark it as having a pending event.  */
 
 static void
-set_pending_status_callback (struct inferior_list_entry *entry)
+set_pending_status_callback (thread_info *thread)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
-
   if (thread->last_status.kind != TARGET_WAITKIND_STOPPED
       || (thread->last_status.value.sig != GDB_SIGNAL_0
 	  /* A breakpoint, watchpoint or finished step from a previous
@@ -3339,10 +3328,8 @@ set_pending_status_callback (struct inferior_list_entry *entry)
    pending status to report to GDB.  */
 
 static int
-find_status_pending_thread_callback (struct inferior_list_entry *entry, void *data)
+find_status_pending_thread_callback (thread_info *thread, void *data)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
-
   return thread->status_pending_p;
 }
 
@@ -3371,7 +3358,7 @@ handle_status (char *own_buf)
     }
   else
     {
-      struct inferior_list_entry *thread = NULL;
+      thread_info *thread = NULL;
 
       pause_all (0);
       stabilize_threads ();
@@ -3401,7 +3388,7 @@ handle_status (char *own_buf)
       /* If we're still out of luck, simply pick the first thread in
 	 the thread list.  */
       if (thread == NULL)
-	thread = get_first_inferior (&all_threads);
+	thread = get_first_thread ();
 
       if (thread != NULL)
 	{
@@ -3417,7 +3404,7 @@ handle_status (char *own_buf)
 	  set_desired_thread ();
 
 	  gdb_assert (tp->last_status.kind != TARGET_WAITKIND_IGNORE);
-	  prepare_resume_reply (own_buf, tp->entry.id, &tp->last_status);
+	  prepare_resume_reply (own_buf, tp->id, &tp->last_status);
 	}
       else
 	strcpy (own_buf, "W00");
@@ -4095,7 +4082,7 @@ process_serial_event (void)
 		  break;
 		}
 
-	      thread_id = thread->entry.id;
+	      thread_id = thread->id;
 	    }
 	  else
 	    {
@@ -4119,7 +4106,7 @@ process_serial_event (void)
 							     general_thread);
 		  if (thread == NULL)
 		    thread = get_first_thread ();
-		  thread_id = thread->entry.id;
+		  thread_id = thread->id;
 		}
 
 	      general_thread = thread_id;
diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index 94224a8a7c..8f757c0a5d 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -55,12 +55,11 @@ struct thread_search
    when accessing memory.  */
 
 static int
-thread_search_callback (struct inferior_list_entry *entry, void *args)
+thread_search_callback (thread_info *thread, void *args)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   struct thread_search *s = (struct thread_search *) args;
 
-  if (ptid_get_pid (entry->id) == ptid_get_pid (s->current_gen_ptid)
+  if (thread->id.pid () == ptid_get_pid (s->current_gen_ptid)
       && mythread_alive (ptid_of (thread)))
     {
       if (s->stopped == NULL
@@ -71,7 +70,7 @@ thread_search_callback (struct inferior_list_entry *entry, void *args)
       if (s->first == NULL)
 	s->first = thread;
 
-      if (s->current == NULL && ptid_equal (s->current_gen_ptid, entry->id))
+      if (s->current == NULL && s->current_gen_ptid == thread->id)
 	s->current = thread;
     }
 
diff --git a/gdb/gdbserver/thread-db.c b/gdb/gdbserver/thread-db.c
index 638a8337b6..67970f4eca 100644
--- a/gdb/gdbserver/thread-db.c
+++ b/gdb/gdbserver/thread-db.c
@@ -407,7 +407,7 @@ thread_db_get_tls_address (struct thread_info *thread, CORE_ADDR offset,
 
   lwp = get_thread_lwp (thread);
   if (!lwp->thread_known)
-    find_one_thread (thread->entry.id);
+    find_one_thread (thread->id);
   if (!lwp->thread_known)
     return TD_NOTHR;
 
@@ -465,7 +465,7 @@ thread_db_thread_handle (ptid_t ptid, gdb_byte **handle, int *handle_len)
 
   lwp = get_thread_lwp (thread);
 
-  if (!lwp->thread_known && !find_one_thread (thread->entry.id))
+  if (!lwp->thread_known && !find_one_thread (thread->id))
     return false;
 
   gdb_assert (lwp->thread_known);
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 8ec8010ac8..73090a753d 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -4396,7 +4396,7 @@ tracepoint_finished_step (struct thread_info *tinfo, CORE_ADDR stop_pc)
   wstep_link = &tinfo->while_stepping;
 
   trace_debug ("Thread %s finished a single-step for tracepoint %d at 0x%s",
-	       target_pid_to_str (tinfo->entry.id),
+	       target_pid_to_str (tinfo->id),
 	       wstep->tp_number, paddress (wstep->tp_address));
 
   ctx.base.type = trap_tracepoint;
@@ -4409,7 +4409,7 @@ tracepoint_finished_step (struct thread_info *tinfo, CORE_ADDR stop_pc)
 	{
 	  trace_debug ("NO TRACEPOINT %d at 0x%s FOR THREAD %s!",
 		       wstep->tp_number, paddress (wstep->tp_address),
-		       target_pid_to_str (tinfo->entry.id));
+		       target_pid_to_str (tinfo->id));
 
 	  /* Unlink.  */
 	  *wstep_link = wstep->next;
@@ -4429,7 +4429,7 @@ tracepoint_finished_step (struct thread_info *tinfo, CORE_ADDR stop_pc)
 	{
 	  /* The requested numbers of steps have occurred.  */
 	  trace_debug ("Thread %s done stepping for tracepoint %d at 0x%s",
-		       target_pid_to_str (tinfo->entry.id),
+		       target_pid_to_str (tinfo->id),
 		       wstep->tp_number, paddress (wstep->tp_address));
 
 	  /* Unlink the wstep.  */
@@ -4576,7 +4576,7 @@ tracepoint_was_hit (struct thread_info *tinfo, CORE_ADDR stop_pc)
 	  && tpoint->type != static_tracepoint)
 	{
 	  trace_debug ("Thread %s at address of tracepoint %d at 0x%s",
-		       target_pid_to_str (tinfo->entry.id),
+		       target_pid_to_str (tinfo->id),
 		       tpoint->number, paddress (tpoint->address));
 
 	  /* Test the condition if present, and collect if true.  */
diff --git a/gdb/gdbserver/win32-i386-low.c b/gdb/gdbserver/win32-i386-low.c
index 03d6e1779d..fd99ca9cb6 100644
--- a/gdb/gdbserver/win32-i386-low.c
+++ b/gdb/gdbserver/win32-i386-low.c
@@ -37,10 +37,8 @@
 static struct x86_debug_reg_state debug_reg_state;
 
 static int
-update_debug_registers_callback (struct inferior_list_entry *entry,
-				 void *pid_p)
+update_debug_registers_callback (thread_info *thr, void *pid_p)
 {
-  struct thread_info *thr = (struct thread_info *) entry;
   win32_thread_info *th = (win32_thread_info *) thread_target_data (thr);
   int pid = *(int *) pid_p;
 
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index f9c890f164..66fc52c392 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -231,9 +231,8 @@ child_add_thread (DWORD pid, DWORD tid, HANDLE h, void *tlb)
 
 /* Delete a thread from the list of threads.  */
 static void
-delete_thread_info (struct inferior_list_entry *entry)
+delete_thread_info (thread_info *thread)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   win32_thread_info *th = (win32_thread_info *) thread_target_data (thread);
 
   remove_thread (thread);
@@ -245,15 +244,14 @@ delete_thread_info (struct inferior_list_entry *entry)
 static void
 child_delete_thread (DWORD pid, DWORD tid)
 {
-  struct inferior_list_entry *thread;
   ptid_t ptid;
 
   /* If the last thread is exiting, just return.  */
-  if (one_inferior_p (&all_threads))
+  if (all_threads.size () == 1)
     return;
 
   ptid = ptid_build (pid, tid, 0);
-  thread = find_inferior_id (&all_threads, ptid);
+  thread_info *thread = find_inferior_id (&all_threads, ptid);
   if (thread == NULL)
     return;
 
@@ -431,9 +429,8 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached)
 /* Resume all artificially suspended threads if we are continuing
    execution.  */
 static int
-continue_one_thread (struct inferior_list_entry *this_thread, void *id_ptr)
+continue_one_thread (thread_info *thread, void *id_ptr)
 {
-  struct thread_info *thread = (struct thread_info *) this_thread;
   int thread_id = * (int *) id_ptr;
   win32_thread_info *th = (win32_thread_info *) thread_target_data (thread);
 
@@ -1347,9 +1344,8 @@ handle_exception (struct target_waitstatus *ourstatus)
 
 
 static void
-suspend_one_thread (struct inferior_list_entry *entry)
+suspend_one_thread (thread_info *thread)
 {
-  struct thread_info *thread = (struct thread_info *) entry;
   win32_thread_info *th = (win32_thread_info *) thread_target_data (thread);
 
   if (!th->suspended)
@@ -1488,7 +1484,7 @@ get_child_debug_event (struct target_waitstatus *ourstatus)
       child_delete_thread (current_event.dwProcessId,
 			   current_event.dwThreadId);
 
-      current_thread = (struct thread_info *) all_threads.head;
+      current_thread = get_first_thread ();
       return 1;
 
     case CREATE_PROCESS_DEBUG_EVENT:
-- 
2.14.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] gdbserver: Use std::list for all_dlls
  2017-10-09 14:30 [PATCH 0/3] Get rid of inferior_list Simon Marchi
  2017-10-09 14:30 ` [PATCH 2/3] gdbserver: Use std::list for all_processes Simon Marchi
  2017-10-09 14:30 ` [PATCH 3/3] gdbserver: use std::list for all_threads Simon Marchi
@ 2017-10-09 14:30 ` Simon Marchi
  2017-10-09 14:44   ` Pedro Alves
  2017-10-14 13:12 ` [PATCH 0/3] Get rid of inferior_list Simon Marchi
  3 siblings, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2017-10-09 14:30 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

As a small step towards removing inferior_list/inferior_list_entry, this
patch replaces the usage of inferior_list for the list of dlls by an
std::list.

I am able to build gdbserver with mingw on Linux, but I am not able to
test this on a Windows machine (the only platform that uses this code).

gdb/gdbserver/ChangeLog:

	* dll.h: Include <list>.
	(struct dll_info) <entry>: Remove field.
	(all_dlls): Change type to std::list<dll_info *>.
	* dll.c: Include <algorithm>.
	(get_dll): Remove macro.
	(all_dlls): Change type to std::list<dll_info *>.
	(free_one_dll): Change parameter type to dll_info *.
	(match_dll): Likewise.
	(loaded_dll): Adjust to removal of entry field and all_dlls type
	change.
	(unloaded_dll): Adjust to all_dlls type change, use
	std::find_if.
	(clear_dlls): Adjust to all_dlls type change.
	* server.c (emit_dll_description): Remove.
	(handle_qxfer_libraries): Adjust to all_dlls type change,
	integrate emit_dll_description's functionality.
---
 gdb/gdbserver/dll.c    | 36 ++++++++++++++++++------------------
 gdb/gdbserver/dll.h    |  8 +++-----
 gdb/gdbserver/server.c | 20 ++++----------------
 3 files changed, 25 insertions(+), 39 deletions(-)

diff --git a/gdb/gdbserver/dll.c b/gdb/gdbserver/dll.c
index c74601a5f4..ec672be797 100644
--- a/gdb/gdbserver/dll.c
+++ b/gdb/gdbserver/dll.c
@@ -18,20 +18,20 @@
 #include "server.h"
 #include "dll.h"
 
-#define get_dll(inf) ((struct dll_info *)(inf))
+#include <algorithm>
 
 /* An "unspecified" CORE_ADDR, for match_dll.  */
 #define UNSPECIFIED_CORE_ADDR (~(CORE_ADDR) 0)
 
-struct inferior_list all_dlls;
+std::list<dll_info *> all_dlls;
 int dlls_changed;
 
 static void
-free_one_dll (struct inferior_list_entry *inf)
+free_one_dll (dll_info *dll)
 {
-  struct dll_info *dll = get_dll (inf);
   if (dll->name != NULL)
     free (dll->name);
+
   free (dll);
 }
 
@@ -39,11 +39,8 @@ free_one_dll (struct inferior_list_entry *inf)
    the key is ignored; so is an all-ones base address.  */
 
 static int
-match_dll (struct inferior_list_entry *inf, void *arg)
+match_dll (dll_info *iter, dll_info *key)
 {
-  struct dll_info *iter = (struct dll_info *) inf;
-  struct dll_info *key = (struct dll_info *) arg;
-
   if (key->base_addr != UNSPECIFIED_CORE_ADDR
       && iter->base_addr == key->base_addr)
     return 1;
@@ -62,12 +59,10 @@ loaded_dll (const char *name, CORE_ADDR base_addr)
 {
   struct dll_info *new_dll = XCNEW (struct dll_info);
 
-  new_dll->entry.id = minus_one_ptid;
-
   new_dll->name = xstrdup (name);
   new_dll->base_addr = base_addr;
 
-  add_inferior_to_list (&all_dlls, &new_dll->entry);
+  all_dlls.push_back (new_dll);
   dlls_changed = 1;
 }
 
@@ -76,16 +71,19 @@ loaded_dll (const char *name, CORE_ADDR base_addr)
 void
 unloaded_dll (const char *name, CORE_ADDR base_addr)
 {
-  struct dll_info *dll;
   struct dll_info key_dll;
 
   /* Be careful not to put the key DLL in any list.  */
   key_dll.name = (char *) name;
   key_dll.base_addr = base_addr;
 
-  dll = (struct dll_info *) find_inferior (&all_dlls, match_dll, &key_dll);
+  auto pred = [&key_dll] (dll_info *dll) {
+    return match_dll (dll, &key_dll);
+  };
 
-  if (dll == NULL)
+  auto dll = std::find_if (all_dlls.begin (), all_dlls.end (), pred);
+
+  if (dll == all_dlls.end ())
     /* For some inferiors we might get unloaded_dll events without having
        a corresponding loaded_dll.  In that case, the dll cannot be found
        in ALL_DLL, and there is nothing further for us to do.
@@ -99,8 +97,8 @@ unloaded_dll (const char *name, CORE_ADDR base_addr)
     {
       /* DLL has been found so remove the entry and free associated
          resources.  */
-      remove_inferior (&all_dlls, &dll->entry);
-      free_one_dll (&dll->entry);
+      free_one_dll (*dll);
+      all_dlls.erase (dll);
       dlls_changed = 1;
     }
 }
@@ -108,6 +106,8 @@ unloaded_dll (const char *name, CORE_ADDR base_addr)
 void
 clear_dlls (void)
 {
-  for_each_inferior (&all_dlls, free_one_dll);
-  clear_inferior_list (&all_dlls);
+  for (dll_info *dll : all_dlls)
+    free_one_dll (dll);
+
+  all_dlls.clear ();
 }
diff --git a/gdb/gdbserver/dll.h b/gdb/gdbserver/dll.h
index 39e5eb0653..52f924bc85 100644
--- a/gdb/gdbserver/dll.h
+++ b/gdb/gdbserver/dll.h
@@ -18,17 +18,15 @@
 #ifndef DLL_H
 #define DLL_H
 
+#include <list>
+
 struct dll_info
 {
-  /* This must appear first.  See inferiors.h.
-     The list iterator functions assume it.  */
-  struct inferior_list_entry entry;
-
   char *name;
   CORE_ADDR base_addr;
 };
 
-extern struct inferior_list all_dlls;
+extern std::list<dll_info *> all_dlls;
 extern int dlls_changed;
 
 extern void clear_dlls (void);
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index a95973547c..3d2c2ff5a0 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1533,21 +1533,6 @@ handle_qxfer_features (const char *annex,
   return len;
 }
 
-/* Worker routine for handle_qxfer_libraries.
-   Emit the XML to describe the library in INF.  */
-
-static void
-emit_dll_description (struct inferior_list_entry *inf, void *arg)
-{
-  struct dll_info *dll = (struct dll_info *) inf;
-  std::string *document = (std::string *) arg;
-  std::string name = xml_escape_text (dll->name);
-
-  *document += string_printf
-    ("  <library name=\"%s\"><segment address=\"0x%lx\"/></library>\n",
-     name.c_str (), (long) dll->base_addr);
-}
-
 /* Handle qXfer:libraries:read.  */
 
 static int
@@ -1563,7 +1548,10 @@ handle_qxfer_libraries (const char *annex,
 
   std::string document = "<library-list version=\"1.0\">\n";
 
-  for_each_inferior_with_data (&all_dlls, emit_dll_description, &document);
+  for (const dll_info *dll : all_dlls)
+    document += string_printf
+      ("  <library name=\"%s\"><segment address=\"0x%lx\"/></library>\n",
+       dll->name, (long) dll->base_addr);
 
   document += "</library-list>\n";
 
-- 
2.14.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] gdbserver: Use std::list for all_dlls
  2017-10-09 14:30 ` [PATCH 1/3] gdbserver: Use std::list for all_dlls Simon Marchi
@ 2017-10-09 14:44   ` Pedro Alves
  2017-10-09 14:49     ` Simon Marchi
  2017-10-09 15:12     ` [PATCH v2] " Simon Marchi
  0 siblings, 2 replies; 10+ messages in thread
From: Pedro Alves @ 2017-10-09 14:44 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 10/09/2017 03:30 PM, Simon Marchi wrote:

> diff --git a/gdb/gdbserver/dll.h b/gdb/gdbserver/dll.h
> index 39e5eb0653..52f924bc85 100644
> --- a/gdb/gdbserver/dll.h
> +++ b/gdb/gdbserver/dll.h
> @@ -18,17 +18,15 @@
>  #ifndef DLL_H
>  #define DLL_H
>  
> +#include <list>
> +
>  struct dll_info
>  {
> -  /* This must appear first.  See inferiors.h.
> -     The list iterator functions assume it.  */
> -  struct inferior_list_entry entry;
> -
>    char *name;
>    CORE_ADDR base_addr;
>  };
>  
> -extern struct inferior_list all_dlls;
> +extern std::list<dll_info *> all_dlls;

Is there a reason for making this a list of dll_info pointers
instead of a list of dll_info objects?  If you make this a list of
objects, then each list node + dll_info is allocated in one go,
very much like the current code.  With a list of pointers, you
have an extra allocation/indirection for each dll_info (one for
node + pointer, another for the dll_info pointee).

Thanks,
Pedro Alves


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] gdbserver: Use std::list for all_dlls
  2017-10-09 14:44   ` Pedro Alves
@ 2017-10-09 14:49     ` Simon Marchi
  2017-10-09 15:12     ` [PATCH v2] " Simon Marchi
  1 sibling, 0 replies; 10+ messages in thread
From: Simon Marchi @ 2017-10-09 14:49 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

On 2017-10-09 10:44 AM, Pedro Alves wrote:
> On 10/09/2017 03:30 PM, Simon Marchi wrote:
> 
>> diff --git a/gdb/gdbserver/dll.h b/gdb/gdbserver/dll.h
>> index 39e5eb0653..52f924bc85 100644
>> --- a/gdb/gdbserver/dll.h
>> +++ b/gdb/gdbserver/dll.h
>> @@ -18,17 +18,15 @@
>>  #ifndef DLL_H
>>  #define DLL_H
>>  
>> +#include <list>
>> +
>>  struct dll_info
>>  {
>> -  /* This must appear first.  See inferiors.h.
>> -     The list iterator functions assume it.  */
>> -  struct inferior_list_entry entry;
>> -
>>    char *name;
>>    CORE_ADDR base_addr;
>>  };
>>  
>> -extern struct inferior_list all_dlls;
>> +extern std::list<dll_info *> all_dlls;
> 
> Is there a reason for making this a list of dll_info pointers
> instead of a list of dll_info objects?  If you make this a list of
> objects, then each list node + dll_info is allocated in one go,
> very much like the current code.  With a list of pointers, you
> have an extra allocation/indirection for each dll_info (one for
> node + pointer, another for the dll_info pointee).

No reason, except that I thought this was a reasonnable intermediary step.
I'll try doing as you say, it's probably not too long.

Simon


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] gdbserver: Use std::list for all_dlls
  2017-10-09 14:44   ` Pedro Alves
  2017-10-09 14:49     ` Simon Marchi
@ 2017-10-09 15:12     ` Simon Marchi
  2017-10-09 15:33       ` Pedro Alves
  1 sibling, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2017-10-09 15:12 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

As a small step towards removing inferior_list/inferior_list_entry, this
patch replaces the usage of inferior_list for the list of dlls by an
std::list.  The dll_info type now uses an std::string for name and has a
simple constructor.

I am able to build gdbserver with mingw on Linux, but I am not able to
test this on a Windows machine (the only platform that uses this code).

gdb/gdbserver/ChangeLog:

	* dll.h: Include <list>.
	(struct dll_info): Add constructor.
	<entry>: Remove field.
	(all_dlls): Change type to std::list<dll_info>.
	* dll.c: Include <algorithm>.
	(get_dll): Remove macro.
	(all_dlls): Change type to std::list<dll_info *>.
	(free_one_dll): Remove.
	(match_dll): Likewise.
	(loaded_dll): Adjust.
	(unloaded_dll): Adjust to all_dlls type change, use
	std::find_if.  Inline code from match_dll.
	(clear_dlls): Adjust to all_dlls type change.
	* server.c (emit_dll_description): Remove.
	(handle_qxfer_libraries): Adjust to all_dlls type change,
	integrate emit_dll_description's functionality.
---
 gdb/gdbserver/dll.c    | 66 ++++++++++++--------------------------------------
 gdb/gdbserver/dll.h    | 12 +++++----
 gdb/gdbserver/server.c | 20 +++------------
 3 files changed, 27 insertions(+), 71 deletions(-)

diff --git a/gdb/gdbserver/dll.c b/gdb/gdbserver/dll.c
index c74601a5f4..718a36df33 100644
--- a/gdb/gdbserver/dll.c
+++ b/gdb/gdbserver/dll.c
@@ -18,56 +18,20 @@
 #include "server.h"
 #include "dll.h"
 
-#define get_dll(inf) ((struct dll_info *)(inf))
+#include <algorithm>
 
 /* An "unspecified" CORE_ADDR, for match_dll.  */
 #define UNSPECIFIED_CORE_ADDR (~(CORE_ADDR) 0)
 
-struct inferior_list all_dlls;
+std::list<dll_info> all_dlls;
 int dlls_changed;
 
-static void
-free_one_dll (struct inferior_list_entry *inf)
-{
-  struct dll_info *dll = get_dll (inf);
-  if (dll->name != NULL)
-    free (dll->name);
-  free (dll);
-}
-
-/* Find a DLL with the same name and/or base address.  A NULL name in
-   the key is ignored; so is an all-ones base address.  */
-
-static int
-match_dll (struct inferior_list_entry *inf, void *arg)
-{
-  struct dll_info *iter = (struct dll_info *) inf;
-  struct dll_info *key = (struct dll_info *) arg;
-
-  if (key->base_addr != UNSPECIFIED_CORE_ADDR
-      && iter->base_addr == key->base_addr)
-    return 1;
-  else if (key->name != NULL
-	   && iter->name != NULL
-	   && strcmp (key->name, iter->name) == 0)
-    return 1;
-
-  return 0;
-}
-
 /* Record a newly loaded DLL at BASE_ADDR.  */
 
 void
 loaded_dll (const char *name, CORE_ADDR base_addr)
 {
-  struct dll_info *new_dll = XCNEW (struct dll_info);
-
-  new_dll->entry.id = minus_one_ptid;
-
-  new_dll->name = xstrdup (name);
-  new_dll->base_addr = base_addr;
-
-  add_inferior_to_list (&all_dlls, &new_dll->entry);
+  all_dlls.emplace_back (name != NULL ? name : "", base_addr);
   dlls_changed = 1;
 }
 
@@ -76,16 +40,20 @@ loaded_dll (const char *name, CORE_ADDR base_addr)
 void
 unloaded_dll (const char *name, CORE_ADDR base_addr)
 {
-  struct dll_info *dll;
-  struct dll_info key_dll;
+  auto pred = [&] (const dll_info &dll) {
+    if (base_addr != UNSPECIFIED_CORE_ADDR
+        && base_addr == dll.base_addr)
+      return true;
+
+    if (name != NULL && dll.name == name)
+      return true;
 
-  /* Be careful not to put the key DLL in any list.  */
-  key_dll.name = (char *) name;
-  key_dll.base_addr = base_addr;
+    return false;
+  };
 
-  dll = (struct dll_info *) find_inferior (&all_dlls, match_dll, &key_dll);
+  auto iter = std::find_if (all_dlls.begin (), all_dlls.end (), pred);
 
-  if (dll == NULL)
+  if (iter == all_dlls.end ())
     /* For some inferiors we might get unloaded_dll events without having
        a corresponding loaded_dll.  In that case, the dll cannot be found
        in ALL_DLL, and there is nothing further for us to do.
@@ -99,8 +67,7 @@ unloaded_dll (const char *name, CORE_ADDR base_addr)
     {
       /* DLL has been found so remove the entry and free associated
          resources.  */
-      remove_inferior (&all_dlls, &dll->entry);
-      free_one_dll (&dll->entry);
+      all_dlls.erase (iter);
       dlls_changed = 1;
     }
 }
@@ -108,6 +75,5 @@ unloaded_dll (const char *name, CORE_ADDR base_addr)
 void
 clear_dlls (void)
 {
-  for_each_inferior (&all_dlls, free_one_dll);
-  clear_inferior_list (&all_dlls);
+  all_dlls.clear ();
 }
diff --git a/gdb/gdbserver/dll.h b/gdb/gdbserver/dll.h
index 39e5eb0653..4956efbe07 100644
--- a/gdb/gdbserver/dll.h
+++ b/gdb/gdbserver/dll.h
@@ -18,17 +18,19 @@
 #ifndef DLL_H
 #define DLL_H
 
+#include <list>
+
 struct dll_info
 {
-  /* This must appear first.  See inferiors.h.
-     The list iterator functions assume it.  */
-  struct inferior_list_entry entry;
+  dll_info (const std::string &name_, CORE_ADDR base_addr_)
+  : name (name_), base_addr (base_addr_)
+  {}
 
-  char *name;
+  std::string name;
   CORE_ADDR base_addr;
 };
 
-extern struct inferior_list all_dlls;
+extern std::list<dll_info> all_dlls;
 extern int dlls_changed;
 
 extern void clear_dlls (void);
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index a95973547c..9f0c186b86 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1533,21 +1533,6 @@ handle_qxfer_features (const char *annex,
   return len;
 }
 
-/* Worker routine for handle_qxfer_libraries.
-   Emit the XML to describe the library in INF.  */
-
-static void
-emit_dll_description (struct inferior_list_entry *inf, void *arg)
-{
-  struct dll_info *dll = (struct dll_info *) inf;
-  std::string *document = (std::string *) arg;
-  std::string name = xml_escape_text (dll->name);
-
-  *document += string_printf
-    ("  <library name=\"%s\"><segment address=\"0x%lx\"/></library>\n",
-     name.c_str (), (long) dll->base_addr);
-}
-
 /* Handle qXfer:libraries:read.  */
 
 static int
@@ -1563,7 +1548,10 @@ handle_qxfer_libraries (const char *annex,
 
   std::string document = "<library-list version=\"1.0\">\n";
 
-  for_each_inferior_with_data (&all_dlls, emit_dll_description, &document);
+  for (const dll_info &dll : all_dlls)
+    document += string_printf
+      ("  <library name=\"%s\"><segment address=\"0x%lx\"/></library>\n",
+       dll.name.c_str (), (long) dll.base_addr);
 
   document += "</library-list>\n";
 
-- 
2.14.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] gdbserver: Use std::list for all_dlls
  2017-10-09 15:12     ` [PATCH v2] " Simon Marchi
@ 2017-10-09 15:33       ` Pedro Alves
  2017-10-09 15:46         ` Simon Marchi
  0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2017-10-09 15:33 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 10/09/2017 04:12 PM, Simon Marchi wrote:
> As a small step towards removing inferior_list/inferior_list_entry, this
> patch replaces the usage of inferior_list for the list of dlls by an
> std::list.  The dll_info type now uses an std::string for name and has a
> simple constructor.
> 
> I am able to build gdbserver with mingw on Linux, but I am not able to
> test this on a Windows machine (the only platform that uses this code).
> 

Thanks for the update.  This version is fine with me.

>  
> @@ -76,16 +40,20 @@ loaded_dll (const char *name, CORE_ADDR base_addr)
>  void
>  unloaded_dll (const char *name, CORE_ADDR base_addr)
>  {
> -  struct dll_info *dll;
> -  struct dll_info key_dll;
> +  auto pred = [&] (const dll_info &dll) {

FWIW, I've been formatting lambdas like this:

     auto pred = [&] (const dll_info &dll)
       {
         if (base_addr != UNSPECIFIED_CORE_ADDR

     for_each (..., [&] (const dll_info &dll)
       {
         if (base_addr != UNSPECIFIED_CORE_ADDR

The latter models on:

     if (foo)
       {
         if (base_addr != UNSPECIFIED_CORE_ADDR

The former I think from the latter, and because that's
what emacs+tab wants.

Thanks,
Pedro Alves


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] gdbserver: Use std::list for all_dlls
  2017-10-09 15:33       ` Pedro Alves
@ 2017-10-09 15:46         ` Simon Marchi
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Marchi @ 2017-10-09 15:46 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

On 2017-10-09 11:33 AM, Pedro Alves wrote:
> On 10/09/2017 04:12 PM, Simon Marchi wrote:
>> As a small step towards removing inferior_list/inferior_list_entry, this
>> patch replaces the usage of inferior_list for the list of dlls by an
>> std::list.  The dll_info type now uses an std::string for name and has a
>> simple constructor.
>>
>> I am able to build gdbserver with mingw on Linux, but I am not able to
>> test this on a Windows machine (the only platform that uses this code).
>>
> 
> Thanks for the update.  This version is fine with me.
> 
>>  
>> @@ -76,16 +40,20 @@ loaded_dll (const char *name, CORE_ADDR base_addr)
>>  void
>>  unloaded_dll (const char *name, CORE_ADDR base_addr)
>>  {
>> -  struct dll_info *dll;
>> -  struct dll_info key_dll;
>> +  auto pred = [&] (const dll_info &dll) {
> 
> FWIW, I've been formatting lambdas like this:
> 
>      auto pred = [&] (const dll_info &dll)
>        {
>          if (base_addr != UNSPECIFIED_CORE_ADDR
> 
>      for_each (..., [&] (const dll_info &dll)
>        {
>          if (base_addr != UNSPECIFIED_CORE_ADDR
> 
> The latter models on:
> 
>      if (foo)
>        {
>          if (base_addr != UNSPECIFIED_CORE_ADDR
> 
> The former I think from the latter, and because that's
> what emacs+tab wants.

Ok, thanks for the tip.  I'll change that in my local branch.

Simon



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] Get rid of inferior_list
  2017-10-09 14:30 [PATCH 0/3] Get rid of inferior_list Simon Marchi
                   ` (2 preceding siblings ...)
  2017-10-09 14:30 ` [PATCH 1/3] gdbserver: Use std::list for all_dlls Simon Marchi
@ 2017-10-14 13:12 ` Simon Marchi
  3 siblings, 0 replies; 10+ messages in thread
From: Simon Marchi @ 2017-10-14 13:12 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 2017-10-09 10:30 AM, Simon Marchi wrote:
> This patchset removes the inferior_list/inferior_list_entry structures,
> in favor of using std::list.  When working in gdbserver, I find it a bit
> cumbersome to work with the threads and processes list, having to cast
> from inferior_list_entry to the real type.
> 
> Simon Marchi (3):
>   gdbserver: Use std::list for all_dlls
>   gdbserver: Use std::list for all_processes
>   gdbserver: use std::list for all_threads
> 
>  gdb/gdbserver/dll.c            |  36 ++---
>  gdb/gdbserver/dll.h            |   8 +-
>  gdb/gdbserver/gdbthread.h      | 109 ++++++++++++++-
>  gdb/gdbserver/inferiors.c      | 301 +++++++++++------------------------------
>  gdb/gdbserver/inferiors.h      | 124 ++++++++---------
>  gdb/gdbserver/linux-arm-low.c  |   5 +-
>  gdb/gdbserver/linux-low.c      | 277 ++++++++++++++++---------------------
>  gdb/gdbserver/linux-mips-low.c |   4 +-
>  gdb/gdbserver/linux-x86-low.c  |  23 ++--
>  gdb/gdbserver/lynx-low.c       |  10 +-
>  gdb/gdbserver/regcache.c       |  18 +--
>  gdb/gdbserver/server.c         | 197 +++++++++------------------
>  gdb/gdbserver/target.c         |   7 +-
>  gdb/gdbserver/thread-db.c      |   4 +-
>  gdb/gdbserver/tracepoint.c     |   8 +-
>  gdb/gdbserver/win32-i386-low.c |   4 +-
>  gdb/gdbserver/win32-low.c      |  16 +--
>  17 files changed, 479 insertions(+), 672 deletions(-)
> 

I pushed this in (including the v2 for patch 1/3).

Simon


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-10-14 13:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-09 14:30 [PATCH 0/3] Get rid of inferior_list Simon Marchi
2017-10-09 14:30 ` [PATCH 2/3] gdbserver: Use std::list for all_processes Simon Marchi
2017-10-09 14:30 ` [PATCH 3/3] gdbserver: use std::list for all_threads Simon Marchi
2017-10-09 14:30 ` [PATCH 1/3] gdbserver: Use std::list for all_dlls Simon Marchi
2017-10-09 14:44   ` Pedro Alves
2017-10-09 14:49     ` Simon Marchi
2017-10-09 15:12     ` [PATCH v2] " Simon Marchi
2017-10-09 15:33       ` Pedro Alves
2017-10-09 15:46         ` Simon Marchi
2017-10-14 13:12 ` [PATCH 0/3] Get rid of inferior_list Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox