Re: [PATCH] merge: use editor by default in interactive sessions

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 23, 2012, 19:23 UTC
Message-ID: <7vd3954ame.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CABPQNSZVOjOKpqv4s1ZCEQRd_yT3us3mqC9aN-KK5PHqztYQQg@mail.gmail.com>

Erik Faye-Lund <kusmabite@gmail.com> writes:

Show 14 quoted lines

>> +       /* Use editor if stdin and stdout are the same and is a tty */
>> +       return (!fstat(0, &st_stdin) &&
>> +               !fstat(1, &st_stdout) &&
>> +               isatty(0) &&
>> +               st_stdin.st_dev == st_stdout.st_dev &&
>> +               st_stdin.st_ino == st_stdout.st_ino &&
>> +               st_stdin.st_mode == st_stdout.st_mode);
>
> I just stumbled over this code, and I got a bit worried; the
> stat-implementation we use on Windows sets st_ino to 0, so
> "st_stdin.st_ino == st_stdout.st_ino" will always evaluate to true.
>
> Perhaps we should add a check for isatty(1) here as well? ...
> Or is there something I'm missing here?

No, the intention was not "we do this whether standard output is tty or not", but was "we check that fd#0 and fd#1 are connected to the same device by trusting stat() to do the right thing, so checking isatty(0) is sufficient". As that "trusting stat()" assumption does not hold for your platform, we would need to add isatty(1) to accomodate it.

Thanks for a set of sharp eyes.