git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Karsten Blees <karsten.blees@gmail.com>
To: Johannes Sixt <j6t@kdbg.org>, Stepan Kasal <kasal@ucw.cz>
Cc: GIT Mailing-list <git@vger.kernel.org>,
	msysGit <msysgit@googlegroups.com>, Karsten Blees <blees@dcon.de>
Subject: [PATCH 7/6] Win32: reliably detect console pipe handles
Date: Sat, 14 Jun 2014 00:09:06 +0200	[thread overview]
Message-ID: <539B7682.7070308@gmail.com> (raw)
In-Reply-To: <539A95F1.9030900@kdbg.org>

As of "Win32: Thread-safe windows console output", child processes may
print to the console even if stdout has been redirected to a file. E.g.:

 git config tar.cat.command "cat"
 git archive -o test.cat HEAD

Detecting whether stdout / stderr point to our console pipe is currently
based on the assumption that OS HANDLE values are never reused. This is
apparently not true if stdout / stderr is replaced via dup2() (as in
builtin/archive.c:17).

Instead of comparing handle values, check if the file descriptor isatty()
backed by a pipe OS handle. This is only possible by swapping the handles
in MSVCRT's internal data structures, as we do in winansi_init().

Reported-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Karsten Blees <blees@dcon.de>
---

Thanks for reporting this.

The fix applies on top of [6/6] Win32: fix broken pipe detection (should
probably not be squashed, as its obviously not as well tested as the
rest of the series).

Cheers,
Karsten


 compat/winansi.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/compat/winansi.c b/compat/winansi.c
index f96d5c2..efc5bb3 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -20,7 +20,6 @@ static WORD attr;
 static int negative;
 static int non_ascii_used = 0;
 static HANDLE hthread, hread, hwrite;
-static HANDLE hwrite1 = INVALID_HANDLE_VALUE, hwrite2 = INVALID_HANDLE_VALUE;
 static HANDLE hconsole1, hconsole2;
 
 #ifdef __MINGW32__
@@ -435,10 +434,6 @@ static void winansi_exit(void)
 	WaitForSingleObject(hthread, INFINITE);
 
 	/* cleanup handles... */
-	if (hwrite1 != INVALID_HANDLE_VALUE)
-		CloseHandle(hwrite1);
-	if (hwrite2 != INVALID_HANDLE_VALUE)
-		CloseHandle(hwrite2);
 	CloseHandle(hwrite);
 	CloseHandle(hthread);
 }
@@ -565,14 +560,9 @@ void winansi_init(void)
 
 	/* redirect stdout / stderr to the pipe */
 	if (con1)
-		hconsole1 = swap_osfhnd(1, hwrite1 = duplicate_handle(hwrite));
+		hconsole1 = swap_osfhnd(1, duplicate_handle(hwrite));
 	if (con2)
-		hconsole2 = swap_osfhnd(2, hwrite2 = duplicate_handle(hwrite));
-}
-
-static int is_same_handle(HANDLE hnd, int fd)
-{
-	return hnd != INVALID_HANDLE_VALUE && hnd == (HANDLE) _get_osfhandle(fd);
+		hconsole2 = swap_osfhnd(2, duplicate_handle(hwrite));
 }
 
 /*
@@ -581,10 +571,9 @@ static int is_same_handle(HANDLE hnd, int fd)
  */
 HANDLE winansi_get_osfhandle(int fd)
 {
-	if (fd == 1 && is_same_handle(hwrite1, 1))
-		return hconsole1;
-	else if (fd == 2 && is_same_handle(hwrite2, 2))
-		return hconsole2;
-	else
-		return (HANDLE) _get_osfhandle(fd);
+	HANDLE hnd = (HANDLE) _get_osfhandle(fd);
+	if ((fd == 1 || fd == 2) && isatty(fd)
+	    && GetFileType(hnd) == FILE_TYPE_PIPE)
+		return (fd == 1) ? hconsole1 : hconsole2;
+	return hnd;
 }
-- 
1.9.3.10.ge3256ea

  reply	other threads:[~2014-06-13 22:09 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-06 13:42 [PATCH 0/5] First part of Unicode console support for msysgit Stepan Kasal
2014-06-06 13:42 ` [PATCH 1/5] Support Unicode console output on Windows Stepan Kasal
2014-06-06 13:42 ` [PATCH 2/5] Detect console streams more reliably " Stepan Kasal
2014-06-06 13:42 ` [PATCH 3/5] Warn if the Windows console font doesn't support Unicode Stepan Kasal
2014-06-06 21:18   ` Peter Krefting
2014-06-07  7:02     ` Stepan Kasal
2014-06-06 13:42 ` [PATCH 4/5] Win32: move main macro to a function Stepan Kasal
2014-06-06 13:42 ` [PATCH 5/5] Win32: Thread-safe windows console output Stepan Kasal
2014-06-06 21:29   ` Peter Krefting
2014-06-06 22:03     ` Karsten Blees
2014-06-06 17:44 ` [PATCH 0/5] First part of Unicode console support for msysgit Karsten Blees
2014-06-06 18:39   ` Stepan Kasal
2014-06-07  7:57     ` [PATCH v2 0/6] " Stepan Kasal
2014-06-07  7:57       ` [PATCH v2 1/6] Support Unicode console output on Windows Stepan Kasal
2014-06-07  7:57       ` [PATCH v2 2/6] Detect console streams more reliably " Stepan Kasal
2014-06-07  7:57       ` [PATCH v2 3/6] Warn if the Windows console font doesn't support Unicode Stepan Kasal
2014-06-07  7:57       ` [PATCH v2 4/6] Win32: add Unicode conversion functions Stepan Kasal
2014-06-07  7:57       ` [PATCH v2 5/6] Win32: Thread-safe windows console output Stepan Kasal
2014-06-13  6:10         ` Johannes Sixt
2014-06-13 22:09           ` Karsten Blees [this message]
2014-06-07  7:57       ` [PATCH v2 6/6] Win32: fix broken pipe detection Stepan Kasal
2014-06-06 20:48   ` Re: [PATCH 0/5] First part of Unicode console support for msysgit Stepan Kasal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=539B7682.7070308@gmail.com \
    --to=karsten.blees@gmail.com \
    --cc=blees@dcon.de \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=kasal@ucw.cz \
    --cc=msysgit@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).