On Thu, Feb 23, 2012 at 9:26 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > >> Erik Faye-Lund <kusmabite@xxxxxxxxx> 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 ... > > s/No,/No, you are not missing anything./; > > I'll queue it with this explanation: > > merge: do not trust fstat(2) too much when checking interactiveness > > The heuristic used by "git merge" to decide if it automatically gives an > editor upon clean automerge is to see if the standard input and the > standard output is the same device and is a tty, we are in an interactive > session. "The same device" test was done by comparing fstat(2) result on > the two file descriptors (and they must match), and we asked isatty() only > for the standard input (we insist that they are the same device and there > is no point asking tty-ness of the standard output). > > The stat(2) emulation on Windows port however does not give a usable value > in st_ino field, so even if the standard output is connected to something Shouldn't that be "emulation _in the_ Windows port" and "in _the_ st_ino field"? > different from the standard input, "The same device" test may incorrectly > return true. To accomodate it, add another isatty() check for the standard > output stream as well. > > Reported-by: Erik Faye-Lund <kusmabite@xxxxxxxxx> > Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> > > Thanks. I just sent a mail with a proper-ish commit message, but I like yours better as it explains the symptom a bit. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html