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>]

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.