Re: [PATCH] Use "switch_to_thread" more thoroughly on gdbserver

[Sergio Durigan Junior <sergiodj at redhat dot com>]

On Friday, September 15 2017, Pedro Alves wrote:

> On 09/15/2017 05:33 AM, Sergio Durigan Junior wrote:
>
>> diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
>> index 752646310a..2226dc802d 100644
>> --- a/gdb/gdbserver/target.c
>> +++ b/gdb/gdbserver/target.c
>> @@ -26,14 +26,11 @@ struct target_ops *the_target;
>>  int
>>  set_desired_thread (int use_general)
>>  {
>> -  struct thread_info *found;
>> -
>>    if (use_general == 1)
>> -    found = find_thread_ptid (general_thread);
>> +    switch_to_thread (general_thread);
>>    else
>> -    found = find_thread_ptid (cont_thread);
>> +    switch_to_thread (cont_thread);
>>  
>> -  current_thread = found;
>>    return (current_thread != NULL);
>
> Did you double-check whether this is equivalent?  Off hand, I'd
> think it is, but the minus_one_ptid check in switch_to_thread
> gave me pause. there's a 'set_continue_thread (minus_one_ptid)' in
> gdb in early connection setup.  Looks like we never call 
> 'set_desired_thread(0)' nowadays though.  [Hc predates vCont].

I think I see what you mean, and I confess I didn't know that you could
set the current thread as minus_one_ptid.

No, I haven't checked if cont_thread can be minus_one_ptid in any moment
in time.  As I said I did some tests here before sending the patch and
nothing came up wrong.

> Do you remember why that check is there in switch_to_thread in the
> first place, as opposed to, say, an assertion?

I tried to do some archaeology here and find the patch that implemented
this feature, and it seems to me that the patch always used 'if' instead
of an 'assert'.  Maybe that's because GDB's version of switch_to_thread
also doesn't assert that ptid != minus_one_ptid...

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/