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
2014-07-09T16:37:21Z

Pedro Alves writes:
> On 07/07/2014 05:39 PM, Doug Evans wrote:
>
> > Our nomenclature here is problematic. I always do a double take when
> > I see attach and target_terminal_foo mentioned in the same sentence.
> > Bleah.
>
> Why? "run; detach; attach SAME_PID" is a trivial case where we end
> up attached to an inferior that is sharing the same tty as GDB. In
> fact, the test does that.

I know.

But for me it's not the normal case.
I wish the naming/wording could better reflect what's happening.
[but that's not something that has to be addressed here of course]

> @@ -2495,10 +2492,26 @@ attach_command (char *args, int from_tty)
> shouldn't refer to attach_target again. */
> attach_target = NULL;
>
> - /* Set up the "saved terminal modes" of the inferior
> - based on what modes we are starting it with. */
> + /* Set up the "saved terminal modes" of the inferior based on what
> + modes we are starting it with. */
> target_terminal_init ();

spurious change

>
> + /* Install inferior's terminal modes. This may look like a no-op,
> + as we've just saved them above, however, this does more than
> + restore terminal settings:
> +
> + - installs a SIGINT handler that forwards SIGINT to the inferior.
> + Otherwise a Ctrl-C pressed just while waiting for that initial
> + stop would end up as a spurious Quit.
> +
> + - removes stdin from the event loop, which we need if attaching
> + in the foreground, otherwise on targets that report an initial
> + stop on attach (which are most) we'd process input/commands
> + while we're in the event loop waiting for that stop. That is,
> + before the attach continuation runs and the command is really
> + finished. */
> + target_terminal_inferior ();
> +
> /* Set up execution context to know that we should return from
> wait_for_inferior as soon as the target reports a stop. */
> init_wait_for_inferior ();

I like this a lot better. Thanks.
The patch is ok with me, modulo removing the spurious change.