threads / patch / 29714

patchmingw: work around stat-limitation

Subject: [PATCH] mingw: work around stat-limitation

## tl;dr

4 messages between Feb 23, 2012 and Feb 23, 2012. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Erik Faye-Lund· Feb 23, 2012, 20:25 UTC · lore

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(-)

Show changes to builtin/merge.c +1 −0

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

Johannes Schindelin· Feb 23, 2012, 21:26 UTC · re: Erik Faye-Lund · lore

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

Hi,

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

Show 8 quoted lines

> 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

Erik Faye-Lund· Feb 23, 2012, 21:36 UTC · re: Johannes Schindelin · lore

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

On Thu, Feb 23, 2012 at 10:26 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 16 quoted lines

> 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.

Johannes Schindelin· Feb 23, 2012, 22:37 UTC · re: Erik Faye-Lund · lore

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

Hi kusma,

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

Show 28 quoted lines

> 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