This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
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]]
- From: Doug Evans <dje at google dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: gdb-patches <gdb-patches at sourceware dot org>
- Date: Wed, 30 Jul 2014 08:34:17 -0700
- Subject: 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]]
- Authentication-results: sourceware.org; auth=none
- References: <1400878753-24688-1-git-send-email-palves at redhat dot com> <538739A2 dot 2050105 at redhat dot com> <20140701162830 dot GA25877 at host2 dot jankratochvil dot net> <1404291574 dot 3766 dot 35 dot camel at bordewijk dot wildebeest dot org> <53B3CDCC dot 9050502 at redhat dot com> <53B57911 dot 10304 at redhat dot com> <53B6B0B8 dot 2050702 at redhat dot com> <21434 dot 52532 dot 737427 dot 778289 at ruffy dot mtv dot corp dot google dot com> <53BC0D0B dot 7040001 at redhat dot com> <21437 dot 28600 dot 751354 dot 629884 at ruffy dot mtv dot corp dot google dot com> <53BD7749 dot 5000800 at redhat dot com> <CADPb22Qt1ctMb9DZg-ftxeAycTqJAkGjtz10ADkoOiDwLggPow at mail dot gmail dot com> <53D8DF7E dot 6070506 at redhat dot 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.