Re: [PATCH v2 06/23] Make thread_still_needs_step_over consider stepping_over_watchpoint too

[Pedro Alves <palves@redhat.com>]

On 04/08/2015 10:28 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> +static int
>> +thread_still_needs_step_over (struct thread_info *tp)
>> +{
>> +  struct inferior *inf = find_inferior_ptid (tp->ptid);
> 
> 'inf' isn't used.
> 

Whoops.  It used to in an older version.  Dropped.

>> @@ -6327,6 +6357,8 @@ keep_going (struct execution_control_state *ecs)
>>        else
>>  	clear_step_over_info ();
>>  
>> +      ecs->event_thread->control.trap_expected = (remove_bp || remove_wps);
>> +
>>        /* Stop stepping if inserting breakpoints fails.  */
>>        TRY
>>  	{
>> @@ -6341,8 +6373,6 @@ keep_going (struct execution_control_state *ecs)
>>  	}
>>        END_CATCH
>>  
>> -      ecs->event_thread->control.trap_expected = (remove_bp || remove_wps);
>> -
> 
> Why do we hoist this line in front of the TRY/CATCH block? because we
> want to set control.trap_expected before insert_breakpoints which may
> throw exception?  We need a ChangeLog entry for it.

Short version: there's really no reason for that. I've undone that last
week, and all the testing I've been doing so far never tripped on
any problem.

The longer version is that in a earlier version of this series (never posted
anywhere), I had a version of keep_going and resume that only prepared the
resume, but did not insert breakpoints or call 'target_resume', so that we could
aggregate all the target_resume calls into a single call.  That
trap_expected set back then was still done by keep_going.  That caused a lot of
pain and so in the end, I undid all that.  When I put back the insert_breakpoints
etc code I probably put it after the trap_expected line instead
of before, probably just because it's easier to read that way, as there's a
connection between that trap_expected set and the code that comes before the
breakpoints insertion.  And then after staring at these patches for
so long, my brain just learned to ignore this diff's hunk as
not important.  :-)

Thanks!

-- 
Pedro Alves