[1/3] gdbserver: Remove duplicate functions to find any thread of process

  We have about 6 functions/callbacks to find_inferior meant to find a
thread that belongs to a given pid.  Remove all but
find_any_thread_of_pid and replace their uses with
find_any_thread_of_pid.

gdbserver/ChangeLog:

	* server.c (first_thread_of): Remove.
	(process_serial_event): Replace usage of first_thread_of with
	find_any_thread_of_pid.
	* tracepoint.c (same_process_p): Remove.
	(gdb_agent_about_to_close): Replace usage of same_process_p with
	find_any_thread_of_pid.
	* linux-x86-low.c (same_process_callback): Remove.
	(x86_arch_setup_process_callback): Replace usage of
	same_process_callback with find_any_thread_of_pid.
	* thread-db.c (any_thread_of): Remove.
	(switch_to_process): Replace usage of any_thread_of with
	find_any_thread_of_pid.
	* inferiors.c (thread_pid_matches_callback): Remove.
	(find_thread_process): Adjust to use find_any_thread_of_pid.
---
 gdb/gdbserver/inferiors.c     | 14 +-------------
 gdb/gdbserver/linux-x86-low.c | 15 +--------------
 gdb/gdbserver/server.c        | 19 +++----------------
 gdb/gdbserver/thread-db.c     | 15 +--------------
 gdb/gdbserver/tracepoint.c    | 14 +-------------
 5 files changed, 7 insertions(+), 70 deletions(-)
  

On Sunday, September 10 2017, Simon Marchi wrote:

> We have about 6 functions/callbacks to find_inferior meant to find a
> thread that belongs to a given pid.  Remove all but
> find_any_thread_of_pid and replace their uses with
> find_any_thread_of_pid.

Thanks for doing.  gdbserver is really confusing when dealing with
threads and their data structures, so this patch is a nice step towards
a better interface.

I think this can go in as is, because it is a self contained cleanup,
but I would like us to move yet another step ahead: recently I've
implemente gdbserver's version of "switch_to_thread", and I couldn't
help but think that a "switch_to_any_thread_of_pid" would be good to
have, because that's what's happening most of the time in this patch.

I know I have to bite the bullet and convert places where
"current_thread = find_thread_ptid (ptid)" is still being used so that
"switch_to_thread" is used globally.  I intend to submit a patch for
that.

Anyway, this looks good to me.

> gdbserver/ChangeLog:
>
> 	* server.c (first_thread_of): Remove.
> 	(process_serial_event): Replace usage of first_thread_of with
> 	find_any_thread_of_pid.
> 	* tracepoint.c (same_process_p): Remove.
> 	(gdb_agent_about_to_close): Replace usage of same_process_p with
> 	find_any_thread_of_pid.
> 	* linux-x86-low.c (same_process_callback): Remove.
> 	(x86_arch_setup_process_callback): Replace usage of
> 	same_process_callback with find_any_thread_of_pid.
> 	* thread-db.c (any_thread_of): Remove.
> 	(switch_to_process): Replace usage of any_thread_of with
> 	find_any_thread_of_pid.
> 	* inferiors.c (thread_pid_matches_callback): Remove.
> 	(find_thread_process): Adjust to use find_any_thread_of_pid.
> ---
>  gdb/gdbserver/inferiors.c     | 14 +-------------
>  gdb/gdbserver/linux-x86-low.c | 15 +--------------
>  gdb/gdbserver/server.c        | 19 +++----------------
>  gdb/gdbserver/thread-db.c     | 15 +--------------
>  gdb/gdbserver/tracepoint.c    | 14 +-------------
>  5 files changed, 7 insertions(+), 70 deletions(-)
>
> diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
> index 3c171a1..2212850 100644
> --- a/gdb/gdbserver/inferiors.c
> +++ b/gdb/gdbserver/inferiors.c
> @@ -141,25 +141,13 @@ find_thread_ptid (ptid_t ptid)
>    return (struct thread_info *) find_inferior_id (&all_threads, ptid);
>  }
>  
> -/* Predicate function for matching thread entry's pid to the given
> -   pid value passed by address in ARGS.  */
> -
> -static int
> -thread_pid_matches_callback (struct inferior_list_entry *entry, void *args)
> -{
> -  return (ptid_get_pid (entry->id) == *(pid_t *)args);
> -}
> -
>  /* Find a thread associated with the given PROCESS, or NULL if no
>     such thread exists.  */
>  
>  static struct thread_info *
>  find_thread_process (const struct process_info *const process)
>  {
> -  pid_t pid = ptid_get_pid (ptid_of (process));
> -
> -  return (struct thread_info *)
> -    find_inferior (&all_threads, thread_pid_matches_callback, &pid);
> +  return find_any_thread_of_pid (process->entry.id.pid ());
>  }
>  
>  /* Helper for find_any_thread_of_pid.  Returns true if a thread
> diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
> index 844a165..49f6b2d 100644
> --- a/gdb/gdbserver/linux-x86-low.c
> +++ b/gdb/gdbserver/linux-x86-low.c
> @@ -846,17 +846,6 @@ x86_linux_read_description (void)
>    gdb_assert_not_reached ("failed to return tdesc");
>  }
>  
> -/* Callback for find_inferior.  Stops iteration when a thread with a
> -   given PID is found.  */
> -
> -static int
> -same_process_callback (struct inferior_list_entry *entry, void *data)
> -{
> -  int pid = *(int *) data;
> -
> -  return (ptid_get_pid (entry->id) == pid);
> -}
> -
>  /* Callback for for_each_inferior.  Calls the arch_setup routine for
>     each process.  */
>  
> @@ -866,9 +855,7 @@ 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
> -    = (struct thread_info *) find_inferior (&all_threads,
> -					    same_process_callback, &pid);
> +  current_thread = find_any_thread_of_pid (pid);
>  
>    the_low_target.arch_setup ();
>  }
> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
> index 56c6393..bedb87b 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -3450,17 +3450,6 @@ gdbserver_show_disableable (FILE *stream)
>        break;					\
>      }
>  
> -static int
> -first_thread_of (struct inferior_list_entry *entry, void *args)
> -{
> -  int pid = * (int *) args;
> -
> -  if (ptid_get_pid (entry->id) == pid)
> -    return 1;
> -
> -  return 0;
> -}
> -
>  static void
>  kill_inferior_callback (struct inferior_list_entry *entry)
>  {
> @@ -4162,11 +4151,9 @@ process_serial_event (void)
>  		   && ptid_equal (pid_to_ptid (pid),
>  				  gdb_id))
>  	    {
> -	      struct thread_info *thread =
> -		(struct thread_info *) find_inferior (&all_threads,
> -						      first_thread_of,
> -						      &pid);
> -	      if (!thread)
> +	      thread_info *thread = find_any_thread_of_pid (pid);
> +
> +	      if (thread == NULL)
>  		{
>  		  write_enn (own_buf);
>  		  break;
> diff --git a/gdb/gdbserver/thread-db.c b/gdb/gdbserver/thread-db.c
> index 1ffb79d..9b867b8 100644
> --- a/gdb/gdbserver/thread-db.c
> +++ b/gdb/gdbserver/thread-db.c
> @@ -733,25 +733,12 @@ thread_db_init (void)
>    return 0;
>  }
>  
> -static int
> -any_thread_of (struct inferior_list_entry *entry, void *args)
> -{
> -  int *pid_p = (int *) args;
> -
> -  if (ptid_get_pid (entry->id) == *pid_p)
> -    return 1;
> -
> -  return 0;
> -}
> -
>  static void
>  switch_to_process (struct process_info *proc)
>  {
>    int pid = pid_of (proc);
>  
> -  current_thread =
> -    (struct thread_info *) find_inferior (&all_threads,
> -					  any_thread_of, &pid);
> +  current_thread = find_any_thread_of_pid (pid);
>  }
>  
>  /* Disconnect from libthread_db and free resources.  */
> diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
> index 68ce10f..0f41ff4 100644
> --- a/gdb/gdbserver/tracepoint.c
> +++ b/gdb/gdbserver/tracepoint.c
> @@ -3956,17 +3956,6 @@ 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 *entry, void *data)
> -{
> -  int *pid = (int *) data;
> -
> -  return ptid_get_pid (entry->id) == *pid;
> -}
> -
>  /* Sent the agent a command to close it.  */
>  
>  void
> @@ -3981,8 +3970,7 @@ gdb_agent_about_to_close (int pid)
>        saved_thread = current_thread;
>  
>        /* Find any thread which belongs to process PID.  */
> -      current_thread = (struct thread_info *)
> -	find_inferior (&all_threads, same_process_p, &pid);
> +      current_thread = find_any_thread_of_pid (pid);
>  
>        strcpy (buf, "close");
>  
> -- 
> 2.7.4
  

On 2017-09-15 06:09, Sergio Durigan Junior wrote:
> On Sunday, September 10 2017, Simon Marchi wrote:
> 
>> We have about 6 functions/callbacks to find_inferior meant to find a
>> thread that belongs to a given pid.  Remove all but
>> find_any_thread_of_pid and replace their uses with
>> find_any_thread_of_pid.
> 
> Thanks for doing.  gdbserver is really confusing when dealing with
> threads and their data structures, so this patch is a nice step towards
> a better interface.

One thing I'm working one is getting rid of 
inferior_list/inferior_list_entry.  The patch is pretty much complete, 
but I keep hesitating about meaningless details... I should just kick 
myself and submit what I have.

> I think this can go in as is, because it is a self contained cleanup,
> but I would like us to move yet another step ahead: recently I've
> implemente gdbserver's version of "switch_to_thread", and I couldn't
> help but think that a "switch_to_any_thread_of_pid" would be good to
> have, because that's what's happening most of the time in this patch.

Ok, sounds like implementing that would just be find_any_thread_of_pid + 
switch_to_thread.

> I know I have to bite the bullet and convert places where
> "current_thread = find_thread_ptid (ptid)" is still being used so that
> "switch_to_thread" is used globally.  I intend to submit a patch for
> that.

Seems like you already did!

> Anyway, this looks good to me.

Thanks for the review, I'll push it in.

Simon