Re: [PATCH] Remove a spurious target_terminal::ours() from windows_nat_target::wait()

[Pedro Alves <palves at redhat dot com>]

On 08/07/2018 03:45 AM, Simon Marchi wrote:
> On 2018-08-02 12:53, Jon Turney wrote:
>> This causes the inferior to stop with SIGTTIN if it tries to read from the
>> terminal after it has been continued.
>>
>> See https://cygwin.com/ml/cygwin/2016-09/msg00285.html for reproduction.
>>
>> Since MinGW doesn't have a tcsetpgrp(), I don't think this problem would be
>> observed there, but Cygwin does so target_terminal::ours() will call it.
>>
>> Calling target_terminal::ours() here seems to be is no longer appropriate
>> after the "Merge async and sync code paths" changes (as the inferior is now
>> in a separate process group even in sync mode(?), which is always used on
>> Windows targets)

I don't really understand what the async/sync code paths changes
here, but regardless ...

>>
>> This call was added in commit c44537cf (and see
>> https://sourceware.org/ml/gdb-patches/2007-02/msg00167.html for what it
>> fixed, which is not regressed by this change)
>>
>> When windows_nat_target::wait() is entered, the inferior is running (either
>> it's been just been started or attached to, or windows_continue() was
>> called), so grabbing the controlling terminal away from it here seems to be
>> wrong, since infrun.c takes care of calling target_terminal::ours() when the
>> inferior stops.
>>
>> gdb/ChangeLog:
>>
>> 2018-08-02  Jon Turney  <jon.turney@dronecode.org.uk>
>>
>>     * windows-nat.c (windows_nat_target::wait): Remove a spurious
>>     target_terminal::ours().
> 
> This seems good to me, but I'd rather check with Pedro, since he knows this stuff much better.

... this LGTM.

Thanks,
Pedro Alves