[PATCH v2 2/3] remove all_lwps from gdbserver: use list api

[Doug Evans <dje at google dot com>]

Hi.
This patch provides missing API routines where necessary,
and removes all references to list implementation details
outside of inferiors.c.

2014-01-28  Doug Evans  <dje@google.com>

	* dll.c (clear_dlls): Replace accessing list implemention details
	with API function.
	* gdbthread.h (get_first_thread): Declare.
	* inferiors.c (for_each_inferior_with_data): New function.
	(get_first_thread): New function.
	(find_thread_ptid): Simplify.
	(get_first_inferior): New function.
	(clear_list): Delete.
	(one_inferior_p): New function.
	(clear_inferior_list): New function.
	(clear_inferiors): Update.
	* inferiors.h (for_each_inferior_with_data): Declare.
	(clear_inferior_list): Declare.
	(one_inferior_p): Declare.
	(get_first_inferior): Declare.
	* server.c (target_running): Replace accessing list implemention
	details with API function.
	(accumulate_file_name_length): New function.
	(emit_dll_description): New function.
	(handle_qxfer_libraries): Replace accessing list implemention
	details with API function.
	(handle_qxfer_threads_worker): New function.
	(handle_qxfer_threads_proper): Replace accessing list implemention
	details with API function.
	(handle_query): Ditto.
	(visit_actioned_threads_callback_ftype): New typedef.
	(visit_actioned_threads_data): New struct.
	(visit_actioned_threads): Rewrite to be find_inferior callback.
	(resume): Call find_inferior.
	(handle_status): Replace accessing list implemention
	details with API function.
	(process_serial_event): Replace accessing list implemention details
	with API function.
	* target.c (set_desired_inferior): Replace accessing list implemention
	details with API function.
	* tracepoint.c (same_process_p): New function.
	(gdb_agent_about_to_close): Replace accessing list implemention
	details with API function.
	* win32-low.c (child_delete_thread): Replace accessing list
	implemention details with API function.
	(match_dll_by_basename): New function.
	(dll_is_loaded_by_basename): New function.
	(win32_ensure_ntdll_loaded): Replace accessing list implemention
	details call to dll_is_loaded_by_basename.

diff --git a/gdb/gdbserver/dll.c b/gdb/gdbserver/dll.c
index be0e01f..52f997c 100644
--- a/gdb/gdbserver/dll.c
+++ b/gdb/gdbserver/dll.c
@@ -110,5 +110,5 @@ void
 clear_dlls (void)
 {
   for_each_inferior (&all_dlls, free_one_dll);
-  all_dlls.head = all_dlls.tail = NULL;
+  clear_inferior_list (&all_dlls);
 }
diff --git a/gdb/gdbserver/gdbthread.h b/gdb/gdbserver/gdbthread.h
index 4c454a2..0eac5a4 100644
--- a/gdb/gdbserver/gdbthread.h
+++ b/gdb/gdbserver/gdbthread.h
@@ -76,6 +76,8 @@ extern struct inferior_list all_threads;
 void remove_thread (struct thread_info *thread);
 void add_thread (ptid_t ptid, void *target_data);
 
+struct thread_info *get_first_thread (void);
+
 struct thread_info *find_thread_ptid (ptid_t ptid);
 
 /* Get current thread ID (Linux task ID).  */
diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
index e3d28ea..a090386 100644
--- a/gdb/gdbserver/inferiors.c
+++ b/gdb/gdbserver/inferiors.c
@@ -59,6 +59,24 @@ for_each_inferior (struct inferior_list *list,
     }
 }
 
+/* Invoke ACTION for each inferior in LIST, passing DATA to ACTION.  */
+
+void
+for_each_inferior_with_data (struct inferior_list *list,
+			     void (*action) (struct inferior_list_entry *,
+					     void *),
+			     void *data)
+{
+  struct inferior_list_entry *cur = list->head, *next;
+
+  while (cur != NULL)
+    {
+      next = cur->next;
+      (*action) (cur, data);
+      cur = next;
+    }
+}
+
 void
 remove_inferior (struct inferior_list *list,
 		 struct inferior_list_entry *entry)
@@ -111,20 +129,18 @@ thread_to_gdb_id (struct thread_info *thread)
   return thread->entry.id;
 }
 
+/* Wrapper around get_first_inferior to return a struct thread_info *.  */
+
 struct thread_info *
-find_thread_ptid (ptid_t ptid)
+get_first_thread (void)
 {
-  struct inferior_list_entry *inf = all_threads.head;
-
-  while (inf != NULL)
-    {
-      struct thread_info *thread = get_thread (inf);
-      if (ptid_equal (thread->entry.id, ptid))
-	return thread;
-      inf = inf->next;
-    }
+  return (struct thread_info *) get_first_inferior ();
+}
 
-  return NULL;
+struct thread_info *
+find_thread_ptid (ptid_t ptid)
+{
+  return (struct thread_info *) find_inferior_id (&all_threads, ptid);
 }
 
 ptid_t
@@ -153,6 +169,19 @@ remove_thread (struct thread_info *thread)
   free_one_thread (&thread->entry);
 }
 
+/* Return a pointer to the first inferior in the 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 (void)
+{
+  if (all_threads.head != NULL)
+    return all_threads.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.  */
 
@@ -214,14 +243,28 @@ set_inferior_regcache_data (struct thread_info *inferior, void *data)
   inferior->regcache_data = data;
 }
 
-#define clear_list(LIST) \
-  do { (LIST)->head = (LIST)->tail = NULL; } while (0)
+/* 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_list (&all_threads);
+  clear_inferior_list (&all_threads);
 
   clear_dlls ();
 
diff --git a/gdb/gdbserver/inferiors.h b/gdb/gdbserver/inferiors.h
index f02afdd..a503be6 100644
--- a/gdb/gdbserver/inferiors.h
+++ b/gdb/gdbserver/inferiors.h
@@ -86,10 +86,21 @@ void add_inferior_to_list (struct inferior_list *list,
 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);
+
 extern struct thread_info *current_inferior;
 void remove_inferior (struct inferior_list *list,
 		      struct inferior_list_entry *entry);
 
+struct inferior_list_entry *get_first_inferior (void);
+
 struct process_info *add_process (int pid, int attached);
 void remove_process (struct process_info *process);
 struct process_info *find_process_pid (int pid);
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 2dc14e8..2190031 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -191,7 +191,7 @@ struct notif_server notif_stop =
 static int
 target_running (void)
 {
-  return all_threads.head != NULL;
+  return get_first_thread () != NULL;
 }
 
 static int
@@ -1135,6 +1135,48 @@ handle_qxfer_features (const char *annex,
   return len;
 }
 
+/* Worker routine for handle_qxfer_libraries.
+   Add to the length pointed to by ARG a conservative estimate of the
+   length needed to transmit the file name of INF.  */
+
+static void
+accumulate_file_name_length (struct inferior_list_entry *inf, void *arg)
+{
+  struct dll_info *dll = (struct dll_info *) inf;
+  unsigned int *total_len = arg;
+
+  /* Over-estimate the necessary memory.  Assume that every character
+     in the library name must be escaped.  */
+  *total_len += 128 + 6 * strlen (dll->name);
+}
+
+/* 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;
+  char **p_ptr = arg;
+  char *p = *p_ptr;
+  char *name;
+
+  strcpy (p, "  <library name=\"");
+  p = p + strlen (p);
+  name = xml_escape_text (dll->name);
+  strcpy (p, name);
+  free (name);
+  p = p + strlen (p);
+  strcpy (p, "\"><segment address=\"");
+  p = p + strlen (p);
+  sprintf (p, "0x%lx", (long) dll->base_addr);
+  p = p + strlen (p);
+  strcpy (p, "\"/></library>\n");
+  p = p + strlen (p);
+
+  *p_ptr = p;
+}
+
 /* Handle qXfer:libraries:read.  */
 
 static int
@@ -1144,7 +1186,6 @@ handle_qxfer_libraries (const char *annex,
 {
   unsigned int total_len;
   char *document, *p;
-  struct inferior_list_entry *dll_ptr;
 
   if (writebuf != NULL)
     return -2;
@@ -1152,11 +1193,9 @@ handle_qxfer_libraries (const char *annex,
   if (annex[0] != '\0' || !target_running ())
     return -1;
 
-  /* Over-estimate the necessary memory.  Assume that every character
-     in the library name must be escaped.  */
   total_len = 64;
-  for (dll_ptr = all_dlls.head; dll_ptr != NULL; dll_ptr = dll_ptr->next)
-    total_len += 128 + 6 * strlen (((struct dll_info *) dll_ptr)->name);
+  for_each_inferior_with_data (&all_dlls, accumulate_file_name_length,
+			       &total_len);
 
   document = malloc (total_len);
   if (document == NULL)
@@ -1165,24 +1204,7 @@ handle_qxfer_libraries (const char *annex,
   strcpy (document, "<library-list>\n");
   p = document + strlen (document);
 
-  for (dll_ptr = all_dlls.head; dll_ptr != NULL; dll_ptr = dll_ptr->next)
-    {
-      struct dll_info *dll = (struct dll_info *) dll_ptr;
-      char *name;
-
-      strcpy (p, "  <library name=\"");
-      p = p + strlen (p);
-      name = xml_escape_text (dll->name);
-      strcpy (p, name);
-      free (name);
-      p = p + strlen (p);
-      strcpy (p, "\"><segment address=\"");
-      p = p + strlen (p);
-      sprintf (p, "0x%lx", (long) dll->base_addr);
-      p = p + strlen (p);
-      strcpy (p, "\"/></library>\n");
-      p = p + strlen (p);
-    }
+  for_each_inferior_with_data (&all_dlls, emit_dll_description, &p);
 
   strcpy (p, "</library-list>\n");
 
@@ -1284,36 +1306,43 @@ handle_qxfer_statictrace (const char *annex,
   return nbytes;
 }
 
-/* Helper for handle_qxfer_threads.  */
+/* Helper for handle_qxfer_threads_proper.
+   Emit the XML to describe the thread of INF.  */
 
 static void
-handle_qxfer_threads_proper (struct buffer *buffer)
+handle_qxfer_threads_worker (struct inferior_list_entry *inf, void *arg)
 {
-  struct inferior_list_entry *thread;
+  struct thread_info *thread = (struct thread_info *) inf;
+  struct buffer *buffer = arg;
+  ptid_t ptid = thread_to_gdb_id (thread);
+  char ptid_s[100];
+  int core = target_core_of_thread (ptid);
+  char core_s[21];
 
-  buffer_grow_str (buffer, "<threads>\n");
+  write_ptid (ptid_s, ptid);
 
-  for (thread = all_threads.head; thread; thread = thread->next)
+  if (core != -1)
     {
-      ptid_t ptid = thread_to_gdb_id ((struct thread_info *)thread);
-      char ptid_s[100];
-      int core = target_core_of_thread (ptid);
-      char core_s[21];
+      sprintf (core_s, "%d", core);
+      buffer_xml_printf (buffer, "<thread id=\"%s\" core=\"%s\"/>\n",
+			 ptid_s, core_s);
+    }
+  else
+    {
+      buffer_xml_printf (buffer, "<thread id=\"%s\"/>\n",
+			 ptid_s);
+    }
+}
 
-      write_ptid (ptid_s, ptid);
+/* Helper for handle_qxfer_threads.  */
 
-      if (core != -1)
-	{
-	  sprintf (core_s, "%d", core);
-	  buffer_xml_printf (buffer, "<thread id=\"%s\" core=\"%s\"/>\n",
-			     ptid_s, core_s);
-	}
-      else
-	{
-	  buffer_xml_printf (buffer, "<thread id=\"%s\"/>\n",
-			     ptid_s);
-	}
-    }
+static void
+handle_qxfer_threads_proper (struct buffer *buffer)
+{
+  buffer_grow_str (buffer, "<threads>\n");
+
+  for_each_inferior_with_data (&all_threads, handle_qxfer_threads_worker,
+			       buffer);
 
   buffer_grow_str0 (buffer, "</threads>\n");
 }
@@ -1701,7 +1730,7 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 	gdb_id = general_thread;
       else
 	{
-	  thread_ptr = all_threads.head;
+	  thread_ptr = get_first_inferior ();
 	  gdb_id = thread_to_gdb_id ((struct thread_info *)thread_ptr);
 	}
 
@@ -1742,7 +1771,7 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 	  ptid_t gdb_id;
 
 	  require_running (own_buf);
-	  thread_ptr = all_threads.head;
+	  thread_ptr = get_first_inferior ();
 
 	  *own_buf++ = 'm';
 	  gdb_id = thread_to_gdb_id ((struct thread_info *)thread_ptr);
@@ -2119,37 +2148,47 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 static void gdb_wants_all_threads_stopped (void);
 static void resume (struct thread_resume *actions, size_t n);
 
+/* The callback that is passed to visit_actioned_threads.  */
+typedef int (visit_actioned_threads_callback_ftype)
+  (const struct thread_resume *, struct thread_info *);
+
+/* Struct to pass data to visit_actioned_threads.  */
+
+struct visit_actioned_threads_data
+{
+  const struct thread_resume *actions;
+  size_t num_actions;
+  visit_actioned_threads_callback_ftype *callback;
+};
+
 /* Call CALLBACK for any thread to which ACTIONS applies to.  Returns
    true if CALLBACK returns true.  Returns false if no matching thread
-   is found or CALLBACK results false.  */
+   is found or CALLBACK results false.
+   Note: This function is itself a callback for find_inferior.  */
 
 static int
-visit_actioned_threads (const struct thread_resume *actions,
-			size_t num_actions,
-			int (*callback) (const struct thread_resume *,
-					 struct thread_info *))
+visit_actioned_threads (struct inferior_list_entry *entry, void *datap)
 {
-  struct inferior_list_entry *entry;
+  struct visit_actioned_threads_data *data = datap;
+  const struct thread_resume *actions = data->actions;
+  size_t num_actions = data->num_actions;
+  visit_actioned_threads_callback_ftype *callback = data->callback;
+  size_t i;
 
-  for (entry = all_threads.head; entry != NULL; entry = entry->next)
+  for (i = 0; i < num_actions; i++)
     {
-      size_t i;
+      const struct thread_resume *action = &actions[i];
 
-      for (i = 0; i < num_actions; i++)
+      if (ptid_equal (action->thread, minus_one_ptid)
+	  || ptid_equal (action->thread, entry->id)
+	  || ((ptid_get_pid (action->thread)
+	       == ptid_get_pid (entry->id))
+	      && ptid_get_lwp (action->thread) == -1))
 	{
-	  const struct thread_resume *action = &actions[i];
-
-	  if (ptid_equal (action->thread, minus_one_ptid)
-	      || ptid_equal (action->thread, entry->id)
-	      || ((ptid_get_pid (action->thread)
-		   == ptid_get_pid (entry->id))
-		  && ptid_get_lwp (action->thread) == -1))
-	    {
-	      struct thread_info *thread = (struct thread_info *) entry;
+	  struct thread_info *thread = (struct thread_info *) entry;
 
-	      if ((*callback) (action, thread))
-		return 1;
-	    }
+	  if ((*callback) (action, thread))
+	    return 1;
 	}
     }
 
@@ -2307,7 +2346,12 @@ resume (struct thread_resume *actions, size_t num_actions)
 	 one with a pending status to report.  If so, skip actually
 	 resuming/stopping and report the pending event
 	 immediately.  */
-      if (visit_actioned_threads (actions, num_actions, handle_pending_status))
+      struct visit_actioned_threads_data data;
+
+      data.actions = actions;
+      data.num_actions = num_actions;
+      data.callback = handle_pending_status;
+      if (find_inferior (&all_threads, visit_actioned_threads, &data) != NULL)
 	return;
 
       enable_async_io ();
@@ -2780,7 +2824,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 = all_threads.head;
+	thread = get_first_inferior ();
 
       if (thread != NULL)
 	{
@@ -3539,7 +3583,10 @@ process_serial_event (void)
 		    (struct thread_info *) find_inferior_id (&all_threads,
 							     general_thread);
 		  if (thread == NULL)
-		    thread_id = all_threads.head->id;
+		    {
+		      thread = get_first_thread ();
+		      thread_id = thread->entry.id;
+		    }
 		}
 
 	      general_thread = thread_id;
diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index e148f33..dcad5c9 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -34,7 +34,7 @@ set_desired_inferior (int use_general)
     found = find_thread_ptid (cont_thread);
 
   if (found == NULL)
-    current_inferior = (struct thread_info *) all_threads.head;
+    current_inferior = get_first_thread ();
   else
     current_inferior = found;
 }
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 3706577..ffbfcff 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -3942,6 +3942,17 @@ cmd_qtstmat (char *packet)
     run_inferior_command (packet, strlen (packet) + 1);
 }
 
+/* Helper for gdb_agent_about_to_close.
+   Return non-zero if thread ENTRY is in the same process in DATA.  */
+
+static int
+same_process_p (struct inferior_list_entry *inf, void *data)
+{
+  int *pid = data;
+
+  return ptid_get_pid (inf->id) == *pid;
+}
+
 /* Sent the agent a command to close it.  */
 
 void
@@ -3952,19 +3963,12 @@ gdb_agent_about_to_close (int pid)
   if (!maybe_write_ipa_not_loaded (buf))
     {
       struct thread_info *save_inferior;
-      struct inferior_list_entry *inf = all_threads.head;
 
       save_inferior = current_inferior;
 
-      /* Find a certain thread which belongs to process PID.  */
-      while (inf != NULL)
-	{
-	  if (ptid_get_pid (inf->id) == pid)
-	    break;
-	  inf = inf->next;
-	}
-
-      current_inferior = (struct thread_info *) inf;
+      /* Find any thread which belongs to process PID.  */
+      current_inferior = (struct thread_info *)
+	find_inferior (&all_threads, same_process_p, &pid);
 
       strcpy (buf, "close");
 
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index f2620a5..e84306d 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -232,7 +232,7 @@ child_delete_thread (DWORD pid, DWORD tid)
   ptid_t ptid;
 
   /* If the last thread is exiting, just return.  */
-  if (all_threads.head == all_threads.tail)
+  if (one_inferior_p (&all_threads))
     return;
 
   ptid = ptid_build (pid, tid, 0);
@@ -1142,6 +1142,28 @@ failed:
 }
 
 #ifndef _WIN32_WCE
+
+/* Helper routine for dll_is_loaded_by_basename.
+   Return non-zero if the basename in ARG matches the DLL in INF.  */
+
+static int
+match_dll_by_basename (struct inferior_list_entry *inf, void *arg)
+{
+  struct dll_info *iter = (void *) inf;
+  const char *basename = arg;
+
+  return strcasecmp (lbasename (iter->name), basename) == 0;
+}
+
+/* Return non-zero if the DLL specified by BASENAME is loaded.  */
+
+static int
+dll_is_loaded_by_basename (const char *basename)
+{
+  return find_inferior (&all_dlls, match_dll_by_basename,
+			(void *) basename) != NULL;
+}
+
 /* On certain versions of Windows, the information about ntdll.dll
    is not available yet at the time we get the LOAD_DLL_DEBUG_EVENT,
    thus preventing us from reporting this DLL as an SO. This has been
@@ -1158,20 +1180,14 @@ failed:
 static void
 win32_ensure_ntdll_loaded (void)
 {
-  struct inferior_list_entry *dll_e;
   size_t i;
   HMODULE dh_buf[1];
   HMODULE *DllHandle = dh_buf;
   DWORD cbNeeded;
   BOOL ok;
 
-  for (dll_e = all_dlls.head; dll_e != NULL; dll_e = dll_e->next)
-    {
-      struct dll_info *dll = (struct dll_info *) dll_e;
-
-      if (strcasecmp (lbasename (dll->name), "ntdll.dll") == 0)
-	return;
-    }
+  if (dll_is_loaded_by_basename ("ntdll.dll"))
+    return;
 
   if (!load_psapi ())
     return;