Re: [PATCH 2/2] test-terminal: drop stdin handling

["Rubén Justo" <rjusto@gmail.com>]

On 6/6/24 10:22 AM, Jeff King wrote:
> Since 18d8c26930 (test_terminal: redirect child process' stdin to a pty,
> 2015-08-04), we set up a pty and copy stdin to the child program. But
> this ends up being racy; once we send all of the bytes and close the
> descriptor, the child program will no longer see a terminal! isatty()
> will return 0, and trying to read may return EIO, even if we didn't yet
> get all of the bytes.
> 
> This was mentioned even in the commit message of 18d8c26930, but we
> hacked around it by just sending an infinite input from /dev/zero (in
> the intended case, we only cared about isatty(0), not reading actual
> input).
> 
> And it came up again recently in:
> 
>   https://lore.kernel.org/git/d42a55b1-1ba9-4cfb-9c3d-98ea4d86da33@gmail.com/
> 
> where we tried to actually send bytes, but they don't always all come
> through. So this interface is somewhat of an accident waiting to happen;
> a caller might not even care about stdin being a tty, but will get bit
> by the flaky behavior.
> 
> One solution would probably be to avoid closing test_terminal's end of
> the pty altogether. But then the other side would never see EOF on its
> stdin.  That may be OK for some cases, but it's another gotcha that
> might cause races or deadlocks, depending on what the child expects to
> read.
> 
> Let's instead just drop test_terminal's stdin feature completely. Since
> the previous commit dropped the two cases from t4153 for which the
> feature was originally added, there are no callers left that need it.
> 
> Signed-off-by: Jeff King <peff@peff.net>

Acked-by: Rubén Justo <rjusto@gmail.com>