All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.