On March 15, 2022 3:05 PM, Junio C Hamano wrote:
>To: rsbecker@xxxxxxxxxxxxx
>Cc: 'Phillip Wood' <phillip.wood123@xxxxxxxxx>; 'Git Mailing List'
><git@xxxxxxxxxxxxxxx>; 'Phillip Wood' <phillip.wood@xxxxxxxxxxxxx>; 'Ævar
>Arnfjörð Bjarmason' <avarab@xxxxxxxxx>; 'Carlo Arenas' <carenas@xxxxxxxxx>;
>'Johannes Schindelin' <Johannes.Schindelin@xxxxxx>; 'Ramsay Jones'
><ramsay@xxxxxxxxxxxxxxxxxxxx>
>Subject: Re: [PATCH v3 2/4] terminal: don't assume stdin is /dev/tty
>
><rsbecker@xxxxxxxxxxxxx> 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).
Thanks. This is one of those situations where explaining magic numbers is important. I appreciate the insight.
Regards,
Randall
