Subject: Re: [PATCH v3 2/4] terminal: don't assume stdin is /dev/tty

From: Junio C Hamano <[email-masked]>
Date: Tue, 15 Mar 2022 12:05:19 -0700

<[email-masked]> writes:

>>The check before closing it is wrong.  It should be
>>
>>	if (0 <= term_fd)
>
> Should this expression succeed if term_fd == stdin? I might be missing the point here.

We could use "if (0 < term_fd)" to make this guard both about
avoiding to call close() on an uninitialized FD and also about
avoiding to close standard input.  I'd prefer to see them handled
separately as these live at different conceptual levels
(i.e. closing -1 is a no-no no matter what, closing 0 is bad if it
is what we did not open but what the caller supplied us via the
SAVE_TERM_STDIN bit, but it may be warranted if it was what we
obtained from an earlier call to open("/dev/tty") we did ourselves).