[PATCH] mingw: work around stat-limitation

[PATCH] mingw: work around stat-limitation

[Erik Faye-Lund]

Our stat implementation for Windows always sets st_ino to 0. This
means that checking if isatty(0) and comparing the reported inodes
of stdout and stdin is not sufficient to detect that both are
pointing to the same TTY.

Luckily, there's only one console on Windows, so adding a check for
isatty(1) should do the trick. For platforms where inodes are
reported correctly, this should still be correct.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---

Here's a proper patch for this glitch.

 builtin/merge.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index ed0f959..bef01e3 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1130,6 +1130,7 @@ static int default_edit_option(void)
 	return (!fstat(0, &st_stdin) &&
 		!fstat(1, &st_stdout) &&
 		isatty(0) &&
+		isatty(1) &&
 		st_stdin.st_dev == st_stdout.st_dev &&
 		st_stdin.st_ino == st_stdout.st_ino &&
 		st_stdin.st_mode == st_stdout.st_mode);
-- 
1.7.9

Re: [msysGit] [PATCH] mingw: work around stat-limitation

[Johannes Schindelin]

Hi,

On Thu, 23 Feb 2012, Erik Faye-Lund wrote:

> Our stat implementation for Windows always sets st_ino to 0. This
> means that checking if isatty(0) and comparing the reported inodes
> of stdout and stdin is not sufficient to detect that both are
> pointing to the same TTY.
> 
> Luckily, there's only one console on Windows, so adding a check for
> isatty(1) should do the trick. For platforms where inodes are
> reported correctly, this should still be correct.

Sorry to ask so stupidly, but why does isatty(1) work and isatty(0) does
not? Should they not access the very same console object?

Thanks,
Dscho

Re: [msysGit] [PATCH] mingw: work around stat-limitation

[Erik Faye-Lund]

On Thu, Feb 23, 2012 at 10:26 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Thu, 23 Feb 2012, Erik Faye-Lund wrote:
>
>> Our stat implementation for Windows always sets st_ino to 0. This
>> means that checking if isatty(0) and comparing the reported inodes
>> of stdout and stdin is not sufficient to detect that both are
>> pointing to the same TTY.
>>
>> Luckily, there's only one console on Windows, so adding a check for
>> isatty(1) should do the trick. For platforms where inodes are
>> reported correctly, this should still be correct.
>
> Sorry to ask so stupidly, but why does isatty(1) work and isatty(0) does
> not? Should they not access the very same console object?
>

The point is that they might not. The old test would draw the wrong
conclusion if isatty(0) was true, but isatty(1) was not because it
used st_ino to verify that stdin and stdout pointed to the same
terminal. The reason it did that was to catch cases where stdin and
stdout pointed to different terminals, AFAICT. Not checking isatty(1)
was simply an optimization, which works when st_ino is filled out
correctly.

On Windows there is only one terminal, so it's sufficient to check if
both are connected.

Re: [msysGit] [PATCH] mingw: work around stat-limitation

[Johannes Schindelin]

Hi kusma,

On Thu, 23 Feb 2012, Erik Faye-Lund wrote:

> On Thu, Feb 23, 2012 at 10:26 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Thu, 23 Feb 2012, Erik Faye-Lund wrote:
> >
> >> Our stat implementation for Windows always sets st_ino to 0. This
> >> means that checking if isatty(0) and comparing the reported inodes
> >> of stdout and stdin is not sufficient to detect that both are
> >> pointing to the same TTY.
> >>
> >> Luckily, there's only one console on Windows, so adding a check for
> >> isatty(1) should do the trick. For platforms where inodes are
> >> reported correctly, this should still be correct.
> >
> > Sorry to ask so stupidly, but why does isatty(1) work and isatty(0) does
> > not? Should they not access the very same console object?
> >
> 
> The point is that they might not. The old test would draw the wrong
> conclusion if isatty(0) was true, but isatty(1) was not because it
> used st_ino to verify that stdin and stdout pointed to the same
> terminal. The reason it did that was to catch cases where stdin and
> stdout pointed to different terminals, AFAICT. Not checking isatty(1)
> was simply an optimization, which works when st_ino is filled out
> correctly.
> 
> On Windows there is only one terminal, so it's sufficient to check if
> both are connected.

Ah, okay. Thanks for answering so patiently!

ACK.

Ciao,
Dscho