From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 23 Feb 2012 19:23:37 GMT
Subject: Re: [PATCH] merge: use editor by default in interactive sessions
Message-ID: <7vd3954ame.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CABPQNSZVOjOKpqv4s1ZCEQRd_yT3us3mqC9aN-KK5PHqztYQQg@mail.gmail.com>

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

>> +       /* 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.