Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]]

[Doug Evans <dje@google.com>]

On Wed, Jul 30, 2014 at 5:05 AM, Pedro Alves <palves@redhat.com> wrote:
>>> [...]
>>> gdb/
>>> 2014-07-09  Pedro Alves  <palves@redhat.com>
>>>
>>>         * infcmd.c (attach_command_post_wait): Don't call
>>>         target_terminal_inferior here.
>>>         (attach_command): Call it here instead.
>>>
>>> gdb/testsuite/
>>> 2014-07-09  Pedro Alves  <palves@redhat.com>
>>>
>>>         * gdb.base/attach-wait-input.exp: New file.
>>>         * gdb.base/attach-wait-input.c: New file.
>>
>> Hi.
>>
>> Is this TODO still needed after this patch?
>>
>> infcmd.c:
>>
>> /*
>>  * TODO:
>>  * Should save/restore the tty state since it might be that the
>>  * program to be debugged was started on this tty and it wants
>>  * the tty in some state other than what we want.  If it's running
>>  * on another terminal or without a terminal, then saving and
>>  * restoring the tty state is a harmless no-op.
>>  * This only needs to be done if we are attaching to a process.
>>  */
>>
>
> As usual, git blame/log is your friend...
>
> That's been in place for over 20 years.   In bd5635a1 (1991), we see:
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /*
>  * TODO:
>  * Should save/restore the tty state since it might be that the
>  * program to be debugged was started on this tty and it wants
>  * the tty in some state other than what we want.  If it's running
>  * on another terminal or without a terminal, then saving and
>  * restoring the tty state is a harmless no-op.
>  * This only needs to be done if we are attaching to a process.
>  */
>
> /*
>  * attach_command --
>  * takes a program started up outside of gdb and ``attaches'' to it.
>  * This stops it cold in its tracks and allows us to start tracing it.
>  * For this to work, we must be able to send the process a
>  * signal and we must have the same effective uid as the program.
>  */
> void
> attach_command (args, from_tty)
>      char *args;
>      int from_tty;
> {
>   target_attach (args, from_tty);
> }
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>
> So clearly the TODO has been stale for a long while.

23 years?  Zoinks! :)

> We've been saving/restoring the tty state way before
> my patch.
>
> Thanks,
> Pedro Alves
>

It wasn't clear to me whether the comment was (trying to) refer to
something more specific for the task at hand.  Just being too literal
I guess.
Thanks.