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

[Simon Marchi <simon dot marchi at polymtl dot ca>]

On 2017-09-15 13:02, Pedro Alves wrote:
> 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.

In the files I can't build, I usually try to be as unintrusive as 
possible, but I'm fine with doing this, it's obvious (tm).

> > @@ -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;
> >  	    }

Ack everything.  The changes are small enough that I don't think it 
warrants a v2.  I'll push directly once the handle_detach refactor is in 
(and post the final versions here).

Thanks!

Simon