Re: [PATCH 2/3] gdbserver: Remove gdb_id_to_thread_id

[Pedro Alves <palves@redhat.com>]

On 09/10/2017 09:11 PM, Simon Marchi wrote:
> From what I understand, this function is not doing anything useful as of
> today.
> 
> Here's the result of my archeological research:
> 

*nod*

> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -4011,7 +4011,6 @@ process_serial_event (void)
>    unsigned int len;
>    int res;
>    CORE_ADDR mem_addr;
> -  int pid;
>    unsigned char sig;
>    int packet_len;
>    int new_packet_len = -1;
> @@ -4039,92 +4038,96 @@ process_serial_event (void)
>        handle_general_set (own_buf);
>        break;
>      case 'D':
> -      require_running (own_buf);
> +      {
> +	require_running (own_buf);
>  

The reindentation makes it hard to see the actual
change.  Is it just moving the int pid variable, or something else?
IMO, it'd be nicer to move the whole case 'D' body to a
handle_detach function.


>      case '!':
>        extended_protocol = 1;
>        write_ok (own_buf);
> @@ -4135,23 +4138,18 @@ process_serial_event (void)
>      case 'H':
>        if (own_buf[1] == 'c' || own_buf[1] == 'g' || own_buf[1] == 's')
>  	{
> -	  ptid_t gdb_id, thread_id;
> -	  int pid;
> +	  ptid_t thread_id;
>  
>  	  require_running (own_buf);
>  
> -	  gdb_id = read_ptid (&own_buf[2], NULL);
> -
> -	  pid = ptid_get_pid (gdb_id);
> +	  thread_id = read_ptid (&own_buf[2], NULL);

Move ptid_t declaration here? :

          ptid_t thread_id = read_ptid (&own_buf[2], NULL);

>  
> -	  if (ptid_equal (gdb_id, null_ptid)
> -	      || ptid_equal (gdb_id, minus_one_ptid))
> +	  if (thread_id == null_ptid || thread_id == minus_one_ptid)
>  	    thread_id = null_ptid;

This can be just:

	  if (thread_id == minus_one_ptid)
 	    thread_id = null_ptid;



> -	  else if (pid != 0
> -		   && ptid_equal (pid_to_ptid (pid),
> -				  gdb_id))
> +	  else if (thread_id.is_pid ())
>  	    {
> -	      thread_info *thread = find_any_thread_of_pid (pid);
> +	      /* The ptid represents a pid.  */
> +	      thread_info *thread = find_any_thread_of_pid (thread_id.pid ());
>  
>  	      if (thread == NULL)
>  		{
> @@ -4163,8 +4161,8 @@ process_serial_event (void)
>  	    }
>  	  else
>  	    {
> -	      thread_id = gdb_id_to_thread_id (gdb_id);
> -	      if (ptid_equal (thread_id, null_ptid))
> +	      /* The ptid represents a lwp/tid.  */
> +	      if (find_thread_ptid (thread_id) == NULL)
>  		{
>  		  write_enn (own_buf);
>  		  break;
> @@ -4369,13 +4367,12 @@ process_serial_event (void)
>  
>      case 'T':
>        {
> -	ptid_t gdb_id, thread_id;
> +	ptid_t thread_id;
>  
>  	require_running (own_buf);
>  
> -	gdb_id = read_ptid (&own_buf[1], NULL);
> -	thread_id = gdb_id_to_thread_id (gdb_id);
> -	if (ptid_equal (thread_id, null_ptid))
> +	thread_id = read_ptid (&own_buf[1], NULL);

ptid_t thread_id = ...   ?

> +	if (find_thread_ptid (thread_id) == NULL)
>  	  {
>  	    write_enn (own_buf);
>  	    break;
> 

Thanks,
Pedro Alves