Re: [PATCH 11/18] gdbserver: fix killed-outside.exp

[Pedro Alves <palves at redhat dot com>]

Hi Yao,

It's taking me a bit to go through your comments.
Thanks for the review, and sorry about that.

On 10/27/2015 09:48 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
>> index 1c447c2..af9ca22 100644
>> --- a/gdb/gdbserver/linux-low.c
>> +++ b/gdb/gdbserver/linux-low.c
>> @@ -1535,12 +1535,6 @@ thread_still_has_status_pending_p (struct thread_info *thread)
>>    if (!lp->status_pending_p)
>>      return 0;
>>  
>> -  /* If we got a `vCont;t', but we haven't reported a stop yet, do
>> -     report any status pending the LWP may have.  */
>> -  if (thread->last_resume_kind == resume_stop
>> -      && thread->last_status.kind != TARGET_WAITKIND_IGNORE)
>> -    return 0;
>> -
> 
> Shouldn't we return 1 instead of 0 here?  This is consistent with the
> comments above.

I may have misunderstood what you mean, but if we remove this code,
that's what we get -- the "return 1;" at the end is reached.

> 
>>    if (thread->last_resume_kind != resume_stop
>>        && (lp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT
>>  	  || lp->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT))
>> @@ -1597,6 +1591,24 @@ thread_still_has_status_pending_p (struct thread_info *thread)
>>    return 1;
>>  }
>>  
>> +/* Returns true if LWP is resumed from the client's perspective.  */
>> +
>> +static int
>> +lwp_resumed (struct lwp_info *lwp)
>> +{
>> +  struct thread_info *thread = get_lwp_thread (lwp);
>> +
>> +  if (thread->last_resume_kind != resume_stop)
>> +    return 1;
>> +
>> +  /* Did we get a `vCont;t', but we haven't reported a stop yet?  */
>> +  if (thread->last_resume_kind == resume_stop
>> +      && thread->last_status.kind == TARGET_WAITKIND_IGNORE)
>> +    return 1;
>> +
> 
> I feel "reported" isn't precise.  Is "got" better?  The code means
> GDBserver has already got vCont;t and sent SIGSTOP, but hasn't *got*
> stop event out of event loop.  After GDBserver got this event, it then
> reports the event back to GDB if needed.

Hmm, reported to gdb is really what I mean.  How about this:

  /* Did gdb send us a `vCont;t', but we haven't reported the
     corresponding stop to gdb yet?  If so, the thread is still
     resumed/running from gdb's perspective.  */
  if (thread->last_resume_kind == resume_stop
      && thread->last_status.kind == TARGET_WAITKIND_IGNORE)
    return 1;

Thanks,
Pedro Alves