Re: [PATCH 3/3] gdbserver: Remove thread_to_gdb_id

[Pedro Alves <palves@redhat.com>]

On 09/10/2017 09:11 PM, Simon Marchi wrote:
> As explained in the previous patch, the gdb_id concept is no longer
> relevant.  The function thread_to_gdb_id is trivial, it returns the
> thread's ptid.  Remove it and replace its usage with ptid_of.
> 
> The changes in nto-low.c and lynx-low.c are fairly straightforward, but
> I was not able to build test them.

Seems fine to me.  Nits below.

> @@ -869,7 +869,7 @@ nto_stopped_by_watchpoint (void)
>      {
>        ptid_t ptid;
>  
> -      ptid = thread_to_gdb_id (current_thread);
> +      ptid = ptid_of (current_thread);
>        if (nto_set_thread (ptid))
>  	{
>  	  const int watchmask = _DEBUG_FLAG_TRACE_RD | _DEBUG_FLAG_TRACE_WR
> @@ -901,7 +901,7 @@ nto_stopped_data_address (void)
>      {
>        ptid_t ptid;
>  
> -      ptid = thread_to_gdb_id (current_thread);
> +      ptid = ptid_of (current_thread);
>  

Above two hunks could merge decl + init.

> @@ -2070,21 +2070,21 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
>    /* Reply the current thread id.  */
>    if (strcmp ("qC", own_buf) == 0 && !disable_packet_qC)
>      {
> -      ptid_t gdb_id;
> +      ptid_t ptid;
>        require_running (own_buf);
>  
>        if (!ptid_equal (general_thread, null_ptid)
>  	  && !ptid_equal (general_thread, minus_one_ptid))

In the other patch you converted equivalent checks to use op!= ,
I don't mind doing it here as well while at it:

        if (general_thread != null_ptid
            && general_thread != minus_one_ptid)


> -	gdb_id = general_thread;
> +	ptid = general_thread;
>        else
>  	{
>  	  thread_ptr = get_first_inferior (&all_threads);
> -	  gdb_id = thread_to_gdb_id ((struct thread_info *)thread_ptr);
> +	  ptid = thread_ptr->id;
>  	}
>  
>        sprintf (own_buf, "QC");
>        own_buf += 2;
> -      write_ptid (own_buf, gdb_id);
> +      write_ptid (own_buf, ptid);
>        return;
>      }
>  
> @@ -2140,28 +2140,24 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
>      {
>        if (strcmp ("qfThreadInfo", own_buf) == 0)
>  	{
> -	  ptid_t gdb_id;
> -
>  	  require_running (own_buf);
>  	  thread_ptr = get_first_inferior (&all_threads);
>  
>  	  *own_buf++ = 'm';
> -	  gdb_id = thread_to_gdb_id ((struct thread_info *)thread_ptr);
> -	  write_ptid (own_buf, gdb_id);
> +	  ptid_t ptid = thread_ptr->id;
> +	  write_ptid (own_buf, ptid);

Could even get rid of the ptid_t variable?

  write_ptid (own_buf, thread_ptr->id);

>  	  thread_ptr = thread_ptr->next;
>  	  return;
>  	}
>  
>        if (strcmp ("qsThreadInfo", own_buf) == 0)
>  	{
> -	  ptid_t gdb_id;
> -
>  	  require_running (own_buf);
>  	  if (thread_ptr != NULL)
>  	    {
>  	      *own_buf++ = 'm';
> -	      gdb_id = thread_to_gdb_id ((struct thread_info *)thread_ptr);
> -	      write_ptid (own_buf, gdb_id);
> +	      ptid_t ptid = thread_ptr->id;
> +	      write_ptid (own_buf, ptid);

Ditto.

>  	      thread_ptr = thread_ptr->next;
>  	      return;
>  	    }
> 

Thanks,
Pedro Alves