[PATCH 0/5] First part of Unicode console support for msysgit

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/5] First part of Unicode console support for msysgit
@ 2014-06-06 13:42 Stepan Kasal
  2014-06-06 13:42 ` [PATCH 1/5] Support Unicode console output on Windows Stepan Kasal
                   ` (5 more replies)
  0 siblings, 6 replies; 22+ messages in thread
From: Stepan Kasal @ 2014-06-06 13:42 UTC (permalink / raw)
  To: GIT Mailing-list; +Cc: msysGit, Stepan Kasal

Hello,

this is first part of the unicode support pathes from msysgit.

The first three patches originate in Jun 2010, though some fixups from 2012
have been squashed in.
The fourth one is just a trivial prerequisite for
the last one, that was written in Jan 2012, with a fixup from Mar 2012.

Regards,
	Stepan

Karsten Blees (5):
  Support Unicode console output on Windows
  Detect console streams more reliably on Windows
  Warn if the Windows console font doesn't support Unicode
  Win32: move main macro to a function
  Win32: Thread-safe windows console output

 compat/mingw.c   |  24 ++-
 compat/mingw.h   |  24 +--
 compat/winansi.c | 446 ++++++++++++++++++++++++++++++++++++++++---------------
 3 files changed, 356 insertions(+), 138 deletions(-)

-- 
2.0.0.9635.g0be03cb

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/5] Support Unicode console output on Windows
  2014-06-06 13:42 [PATCH 0/5] First part of Unicode console support for msysgit Stepan Kasal
@ 2014-06-06 13:42 ` Stepan Kasal
  2014-06-06 13:42 ` [PATCH 2/5] Detect console streams more reliably " Stepan Kasal
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Stepan Kasal @ 2014-06-06 13:42 UTC (permalink / raw)
  To: GIT Mailing-list
  Cc: msysGit, Karsten Blees, Johannes Schindelin, Stepan Kasal

From: Karsten Blees <blees@dcon.de>

WriteConsoleW seems to be the only way to reliably print unicode to the
console (without weird code page conversions).

Also redirects vfprintf to the winansi.c version.

Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Stepan Kasal <kasal@ucw.cz>
---
 compat/mingw.h   |  2 ++
 compat/winansi.c | 26 ++++++++++++++++++++------
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/compat/mingw.h b/compat/mingw.h
index 3eaf822..a465d1e 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -320,9 +320,11 @@ int mingw_raise(int sig);
 int winansi_fputs(const char *str, FILE *stream);
 int winansi_printf(const char *format, ...) __attribute__((format (printf, 1, 2)));
 int winansi_fprintf(FILE *stream, const char *format, ...) __attribute__((format (printf, 2, 3)));
+int winansi_vfprintf(FILE *stream, const char *format, va_list list);
 #define fputs winansi_fputs
 #define printf(...) winansi_printf(__VA_ARGS__)
 #define fprintf(...) winansi_fprintf(__VA_ARGS__)
+#define vfprintf winansi_vfprintf
 
 /*
  * git specific compatibility
diff --git a/compat/winansi.c b/compat/winansi.c
index dedce21..abe0fea 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -3,6 +3,7 @@
  */
 
 #include "../git-compat-util.h"
+#include <malloc.h>
 
 /*
  Functions to be wrapped:
@@ -10,6 +11,7 @@
 #undef printf
 #undef fprintf
 #undef fputs
+#undef vfprintf
 /* TODO: write */
 
 /*
@@ -46,6 +48,18 @@ static void init(void)
 	initialized = 1;
 }
 
+static int write_console(const char *str, size_t len)
+{
+	/* convert utf-8 to utf-16, write directly to console */
+	int wlen = MultiByteToWideChar(CP_UTF8, 0, str, len, NULL, 0);
+	wchar_t *wbuf = (wchar_t *) alloca(wlen * sizeof(wchar_t));
+	MultiByteToWideChar(CP_UTF8, 0, str, len, wbuf, wlen);
+
+	WriteConsoleW(console, wbuf, wlen, NULL, NULL);
+
+	/* return original (utf-8 encoded) length */
+	return len;
+}
 
 #define FOREGROUND_ALL (FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_BLUE)
 #define BACKGROUND_ALL (BACKGROUND_RED | BACKGROUND_GREEN | BACKGROUND_BLUE)
@@ -245,13 +259,15 @@ static int ansi_emulate(const char *str, FILE *stream)
 	int rv = 0;
 	const char *pos = str;
 
+	fflush(stream);
+
 	while (*pos) {
 		pos = strstr(str, "\033[");
 		if (pos) {
 			size_t len = pos - str;
 
 			if (len) {
-				size_t out_len = fwrite(str, 1, len, stream);
+				size_t out_len = write_console(str, len);
 				rv += out_len;
 				if (out_len < len)
 					return rv;
@@ -260,14 +276,12 @@ static int ansi_emulate(const char *str, FILE *stream)
 			str = pos + 2;
 			rv += 2;
 
-			fflush(stream);
-
 			pos = set_attr(str);
 			rv += pos - str;
 			str = pos;
 		} else {
-			rv += strlen(str);
-			fputs(str, stream);
+			size_t len = strlen(str);
+			rv += write_console(str, len);
 			return rv;
 		}
 	}
@@ -294,7 +308,7 @@ int winansi_fputs(const char *str, FILE *stream)
 		return EOF;
 }
 
-static int winansi_vfprintf(FILE *stream, const char *format, va_list list)
+int winansi_vfprintf(FILE *stream, const char *format, va_list list)
 {
 	int len, rv;
 	char small_buf[256];
-- 
2.0.0.9635.g0be03cb

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/5] Detect console streams more reliably on Windows
  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 ` Stepan Kasal
  2014-06-06 13:42 ` [PATCH 3/5] Warn if the Windows console font doesn't support Unicode Stepan Kasal
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Stepan Kasal @ 2014-06-06 13:42 UTC (permalink / raw)
  To: GIT Mailing-list
  Cc: msysGit, Karsten Blees, Johannes Schindelin, Stepan Kasal

From: Karsten Blees <blees@dcon.de>

GetStdHandle(STD_OUTPUT_HANDLE) doesn't work for stderr if stdout is
redirected. Use _get_osfhandle of the FILE* instead.

_isatty() is true for all character devices (including parallel and serial
ports). Check return value of GetConsoleScreenBufferInfo instead to
reliably detect console handles (also don't initialize internal state from
an uninitialized CONSOLE_SCREEN_BUFFER_INFO structure if the function
fails).

Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Stepan Kasal <kasal@ucw.cz>
---
 compat/winansi.c | 50 ++++++++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/compat/winansi.c b/compat/winansi.c
index abe0fea..c4be401 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -25,27 +25,39 @@ static HANDLE console;
 static WORD plain_attr;
 static WORD attr;
 static int negative;
+static FILE *last_stream = NULL;
 
-static void init(void)
+static int is_console(FILE *stream)
 {
 	CONSOLE_SCREEN_BUFFER_INFO sbi;
+	HANDLE hcon;
 
 	static int initialized = 0;
-	if (initialized)
-		return;
 
-	console = GetStdHandle(STD_OUTPUT_HANDLE);
-	if (console == INVALID_HANDLE_VALUE)
-		console = NULL;
+	/* use cached value if stream hasn't changed */
+	if (stream == last_stream)
+		return console != NULL;
 
-	if (!console)
-		return;
+	last_stream = stream;
+	console = NULL;
 
-	GetConsoleScreenBufferInfo(console, &sbi);
-	attr = plain_attr = sbi.wAttributes;
-	negative = 0;
+	/* get OS handle of the stream */
+	hcon = (HANDLE) _get_osfhandle(_fileno(stream));
+	if (hcon == INVALID_HANDLE_VALUE)
+		return 0;
+
+	/* check if its a handle to a console output screen buffer */
+	if (!GetConsoleScreenBufferInfo(hcon, &sbi))
+		return 0;
+
+	if (!initialized) {
+		attr = plain_attr = sbi.wAttributes;
+		negative = 0;
+		initialized = 1;
+	}
 
-	initialized = 1;
+	console = hcon;
+	return 1;
 }
 
 static int write_console(const char *str, size_t len)
@@ -292,12 +304,7 @@ int winansi_fputs(const char *str, FILE *stream)
 {
 	int rv;
 
-	if (!isatty(fileno(stream)))
-		return fputs(str, stream);
-
-	init();
-
-	if (!console)
+	if (!is_console(stream))
 		return fputs(str, stream);
 
 	rv = ansi_emulate(str, stream);
@@ -315,12 +322,7 @@ int winansi_vfprintf(FILE *stream, const char *format, va_list list)
 	char *buf = small_buf;
 	va_list cp;
 
-	if (!isatty(fileno(stream)))
-		goto abort;
-
-	init();
-
-	if (!console)
+	if (!is_console(stream))
 		goto abort;
 
 	va_copy(cp, list);
-- 
2.0.0.9635.g0be03cb

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/5] Warn if the Windows console font doesn't support Unicode
  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 ` Stepan Kasal
  2014-06-06 21:18   ` Peter Krefting
  2014-06-06 13:42 ` [PATCH 4/5] Win32: move main macro to a function Stepan Kasal
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 22+ messages in thread
From: Stepan Kasal @ 2014-06-06 13:42 UTC (permalink / raw)
  To: GIT Mailing-list
  Cc: msysGit, Karsten Blees, Johannes Schindelin, Stepan Kasal

From: Karsten Blees <blees@dcon.de>

Unicode console output won't display correctly with default settings
because the default console font ("Terminal") only supports the system's
OEM charset. Unfortunately, this is a user specific setting, so it cannot
be easily fixed by e.g. some registry tricks in the setup program.

This change prints a warning on exit if console output contained non-ascii
characters and the console font is supposedly not a TrueType font (which
usually have decent Unicode support).

Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Stepan Kasal <kasal@ucw.cz>
---
 compat/winansi.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/compat/winansi.c b/compat/winansi.c
index c4be401..bec6713 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -2,8 +2,11 @@
  * Copyright 2008 Peter Harris <git@peter.is-a-geek.org>
  */
 
+#undef NOGDI
 #include "../git-compat-util.h"
 #include <malloc.h>
+#include <wingdi.h>
+#include <winreg.h>
 
 /*
  Functions to be wrapped:
@@ -27,6 +30,62 @@ static WORD attr;
 static int negative;
 static FILE *last_stream = NULL;
 
+#ifdef __MINGW32__
+typedef struct _CONSOLE_FONT_INFOEX {
+	ULONG cbSize;
+	DWORD nFont;
+	COORD dwFontSize;
+	UINT FontFamily;
+	UINT FontWeight;
+	WCHAR FaceName[LF_FACESIZE];
+} CONSOLE_FONT_INFOEX, *PCONSOLE_FONT_INFOEX;
+#endif
+
+typedef BOOL (WINAPI *PGETCURRENTCONSOLEFONTEX)(HANDLE, BOOL,
+		PCONSOLE_FONT_INFOEX);
+
+static void print_font_warning(void)
+{
+	warning("Your console font probably doesn\'t support Unicode. If "
+		"you experience strange characters in the output, consider "
+		"switching to a TrueType font such as Lucida Console!");
+}
+
+static void check_truetype_font(void)
+{
+	static int truetype_font_checked;
+	DWORD fontFamily = 0;
+	PGETCURRENTCONSOLEFONTEX pGetCurrentConsoleFontEx;
+
+	/* don't do this twice */
+	if (truetype_font_checked)
+		return;
+	truetype_font_checked = 1;
+
+	/* GetCurrentConsoleFontEx is available since Vista */
+	pGetCurrentConsoleFontEx = (PGETCURRENTCONSOLEFONTEX) GetProcAddress(
+			GetModuleHandle("kernel32.dll"), "GetCurrentConsoleFontEx");
+	if (pGetCurrentConsoleFontEx) {
+		CONSOLE_FONT_INFOEX cfi;
+		cfi.cbSize = sizeof(cfi);
+		if (pGetCurrentConsoleFontEx(console, 0, &cfi))
+			fontFamily = cfi.FontFamily;
+	} else {
+		/* pre-Vista: check default console font in registry */
+		HKEY hkey;
+		if (ERROR_SUCCESS == RegOpenKeyExA(HKEY_CURRENT_USER, "Console", 0,
+				KEY_READ, &hkey)) {
+			DWORD size = sizeof(fontFamily);
+			RegQueryValueExA(hkey, "FontFamily", NULL, NULL,
+					(LPVOID) &fontFamily, &size);
+			RegCloseKey(hkey);
+		}
+	}
+
+	if (!(fontFamily & TMPF_TRUETYPE))
+		atexit(print_font_warning);
+}
+
 static int is_console(FILE *stream)
 {
 	CONSOLE_SCREEN_BUFFER_INFO sbi;
@@ -69,6 +128,13 @@ static int write_console(const char *str, size_t len)
 
 	WriteConsoleW(console, wbuf, wlen, NULL, NULL);
 
+	/*
+	 * if non-ascii characters are printed, check that the current console
+	 * font supports this
+	 */
+	if (wlen != len)
+		check_truetype_font();
+
 	/* return original (utf-8 encoded) length */
 	return len;
 }
-- 
2.0.0.9635.g0be03cb

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/5] Warn if the Windows console font doesn't support Unicode
  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
  0 siblings, 1 reply; 22+ messages in thread
From: Peter Krefting @ 2014-06-06 21:18 UTC (permalink / raw)
  To: Stepan Kasal
  Cc: GIT Mailing-list, msysGit, Karsten Blees, Johannes Schindelin

Stepan Kasal:

> +	warning("Your console font probably doesn\'t support Unicode. If "
> +		"you experience strange characters in the output, consider "
> +		"switching to a TrueType font such as Lucida Console!");

As you mention this is an old patch series, but I would recommend 
modernizing the suggestion here to recomment "Consolas". It is 
available in all current versions of Windows (Vista and later), and 
seem to have better Unicode support according to 
<http://www.fileformat.info/info/unicode/font/consolas/list.htm> vs 
<http://www.fileformat.info/info/unicode/font/lucida_console/list.htm>

-- 
\\// Peter - http://www.softwolves.pp.se/

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Re: [PATCH 3/5] Warn if the Windows console font doesn't support Unicode
  2014-06-06 21:18   ` Peter Krefting
@ 2014-06-07  7:02     ` Stepan Kasal
  0 siblings, 0 replies; 22+ messages in thread
From: Stepan Kasal @ 2014-06-07  7:02 UTC (permalink / raw)
  To: Peter Krefting
  Cc: GIT Mailing-list, msysGit, Karsten Blees, Johannes Schindelin

Hi,

On Fri, Jun 06, 2014 at 10:18:43PM +0100, Peter Krefting wrote:
> Stepan Kasal:
>> +		"switching to a TrueType font such as Lucida Console!");
[...]
> modernizing the suggestion here to recomment "Consolas". It is available 

Indeed.

So, I'll keep this patch as it is, for the records, and apply your
suggestion to the subsequent
	[PATCH 5/5] Win32: Thread-safe windows console output

Stepan

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 4/5] Win32: move main macro to a function
  2014-06-06 13:42 [PATCH 0/5] First part of Unicode console support for msysgit Stepan Kasal
                   ` (2 preceding siblings ...)
  2014-06-06 13:42 ` [PATCH 3/5] Warn if the Windows console font doesn't support Unicode Stepan Kasal
@ 2014-06-06 13:42 ` Stepan Kasal
  2014-06-06 13:42 ` [PATCH 5/5] Win32: Thread-safe windows console output Stepan Kasal
  2014-06-06 17:44 ` [PATCH 0/5] First part of Unicode console support for msysgit Karsten Blees
  5 siblings, 0 replies; 22+ messages in thread
From: Stepan Kasal @ 2014-06-06 13:42 UTC (permalink / raw)
  To: GIT Mailing-list; +Cc: msysGit, Karsten Blees, Erik Faye-Lund, Stepan Kasal

From: Karsten Blees <blees@dcon.de>

The code in the MinGW main macro is getting more and more complex, move to
a separate initialization function for readabiliy and extensibility.

Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
Signed-off-by: Stepan Kasal <kasal@ucw.cz>
---
 compat/mingw.c | 15 +++++++++++++++
 compat/mingw.h | 14 ++++----------
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index a0e13bc..c03bafa 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1847,3 +1847,18 @@ int mingw_offset_1st_component(const char *path)
 
 	return offset + is_dir_sep(path[offset]);
 }
+
+void mingw_startup()
+{
+	/* copy executable name to argv[0] */
+	__argv[0] = xstrdup(_pgmptr);
+
+	/* initialize critical section for waitpid pinfo_t list */
+	InitializeCriticalSection(&pinfo_cs);
+
+	/* set up default file mode and file modes for stdin/out/err */
+	_fmode = _O_BINARY;
+	_setmode(_fileno(stdin), _O_BINARY);
+	_setmode(_fileno(stdout), _O_BINARY);
+	_setmode(_fileno(stderr), _O_BINARY);
+}
diff --git a/compat/mingw.h b/compat/mingw.h
index a465d1e..96d15ca 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -365,22 +365,16 @@ void free_environ(char **env);
 extern CRITICAL_SECTION pinfo_cs;
 
 /*
- * A replacement of main() that ensures that argv[0] has a path
- * and that default fmode and std(in|out|err) are in binary mode
+ * A replacement of main() that adds win32 specific initialization.
  */
 
+void mingw_startup();
 #define main(c,v) dummy_decl_mingw_main(); \
 static int mingw_main(c,v); \
 int main(int argc, char **argv) \
 { \
-	extern CRITICAL_SECTION pinfo_cs; \
-	_fmode = _O_BINARY; \
-	_setmode(_fileno(stdin), _O_BINARY); \
-	_setmode(_fileno(stdout), _O_BINARY); \
-	_setmode(_fileno(stderr), _O_BINARY); \
-	argv[0] = xstrdup(_pgmptr); \
-	InitializeCriticalSection(&pinfo_cs); \
-	return mingw_main(argc, argv); \
+	mingw_startup(); \
+	return mingw_main(__argc, __argv); \
 } \
 static int mingw_main(c,v)
 
-- 
2.0.0.9635.g0be03cb

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 5/5] Win32: Thread-safe windows console output
  2014-06-06 13:42 [PATCH 0/5] First part of Unicode console support for msysgit Stepan Kasal
                   ` (3 preceding siblings ...)
  2014-06-06 13:42 ` [PATCH 4/5] Win32: move main macro to a function Stepan Kasal
@ 2014-06-06 13:42 ` Stepan Kasal
  2014-06-06 21:29   ` Peter Krefting
  2014-06-06 17:44 ` [PATCH 0/5] First part of Unicode console support for msysgit Karsten Blees
  5 siblings, 1 reply; 22+ messages in thread
From: Stepan Kasal @ 2014-06-06 13:42 UTC (permalink / raw)
  To: GIT Mailing-list; +Cc: msysGit, Karsten Blees, Stepan Kasal

From: Karsten Blees <blees@dcon.de>

Winansi.c has many static variables that are accessed and modified from
the [v][f]printf / fputs functions overridden in the file. This may cause
multi threaded git commands that print to the console to produce corrupted
output or even crash.

Additionally, winansi.c doesn't override all functions that can be used to
print to the console (e.g. fwrite, write, fputc are missing), so that ANSI
escapes don't work properly for some git commands (e.g. git-grep).

Instead of doing ANSI emulation in just a few wrapped functions on top of
the IO API, let's plug into the IO system and take advantage of the thread
safety inherent to the IO system.

Redirect stdout and stderr to a pipe if they point to the console. A
background thread reads from the pipe, handles ANSI escape sequences and
UTF-8 to UTF-16 conversion, then writes to the console.

The pipe-based stdout and stderr replacements must be set to unbuffered, as
MSVCRT doesn't support line buffering and fully buffered streams are
inappropriate for console output.

Due to the byte-oriented pipe, ANSI escape sequences and multi-byte UTF-8
sequences can no longer be expected to arrive in one piece. Replace the
string-based ansi_emulate() with a simple stateful parser (this also fixes
colored diff hunk headers, which were broken as of commit 2efcc977).

Override isatty to return true for the pipes redirecting to the console.

Exec/spawn obtain the original console handle to pass to the next process
via winansi_get_osfhandle().

All other overrides are gone, the default stdio implementations work as
expected with the piped stdout/stderr descriptors.

Global variables are either initialized on startup (single threaded) or
exclusively modified by the background thread. Threads communicate through
the pipe, no further synchronization is necessary.

The background thread is terminated by disonnecting the pipe after flushing
the stdio and pipe buffers. This doesn't work for anonymous pipes (created
via CreatePipe), as DisconnectNamedPipe only works on the read end, which
discards remaining data. Thus we have to setup the pipe manually, with the
write end beeing the server (opened with CreateNamedPipe) and the read end
the client (opened with CreateFile).

Limitations: doesn't track reopened or duped file descriptors, i.e.:
- fdopen(1/2) returns fully buffered streams
- dup(1/2), dup2(1/2) returns normal pipe descriptors (i.e. isatty() =
  false, winansi_get_osfhandle won't return the original console handle)

Currently, only the git-format-patch command uses xfdopen(xdup(1)) (see
"realstdout" in builtin/log.c), but works well with these limitations.

Many thanks to Atsushi Nakagawa <atnak@chejz.com> for suggesting and
reviewing the thread-exit-mechanism.

Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Stepan Kasal <kasal@ucw.cz>
---
 compat/mingw.c   |   9 +-
 compat/mingw.h   |  12 +-
 compat/winansi.c | 402 ++++++++++++++++++++++++++++++++++++-------------------
 3 files changed, 274 insertions(+), 149 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index c03bafa..831043e 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -865,9 +865,9 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **env,
 	memset(&si, 0, sizeof(si));
 	si.cb = sizeof(si);
 	si.dwFlags = STARTF_USESTDHANDLES;
-	si.hStdInput = (HANDLE) _get_osfhandle(fhin);
-	si.hStdOutput = (HANDLE) _get_osfhandle(fhout);
-	si.hStdError = (HANDLE) _get_osfhandle(fherr);
+	si.hStdInput = winansi_get_osfhandle(fhin);
+	si.hStdOutput = winansi_get_osfhandle(fhout);
+	si.hStdError = winansi_get_osfhandle(fherr);
 
 	/* concatenate argv, quoting args as we go */
 	strbuf_init(&args, 0);
@@ -1861,4 +1861,7 @@ void mingw_startup()
 	_setmode(_fileno(stdin), _O_BINARY);
 	_setmode(_fileno(stdout), _O_BINARY);
 	_setmode(_fileno(stderr), _O_BINARY);
+
+	/* initialize Unicode console */
+	winansi_init();
 }
diff --git a/compat/mingw.h b/compat/mingw.h
index 96d15ca..82e75d3 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -317,14 +317,10 @@ int mingw_raise(int sig);
  * ANSI emulation wrappers
  */
 
-int winansi_fputs(const char *str, FILE *stream);
-int winansi_printf(const char *format, ...) __attribute__((format (printf, 1, 2)));
-int winansi_fprintf(FILE *stream, const char *format, ...) __attribute__((format (printf, 2, 3)));
-int winansi_vfprintf(FILE *stream, const char *format, va_list list);
-#define fputs winansi_fputs
-#define printf(...) winansi_printf(__VA_ARGS__)
-#define fprintf(...) winansi_fprintf(__VA_ARGS__)
-#define vfprintf winansi_vfprintf
+void winansi_init(void);
+int winansi_isatty(int fd);
+HANDLE winansi_get_osfhandle(int fd);
+#define isatty winansi_isatty
 
 /*
  * git specific compatibility
diff --git a/compat/winansi.c b/compat/winansi.c
index bec6713..598fa1a 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -4,18 +4,13 @@
 
 #undef NOGDI
 #include "../git-compat-util.h"
-#include <malloc.h>
 #include <wingdi.h>
 #include <winreg.h>
 
 /*
  Functions to be wrapped:
 */
-#undef printf
-#undef fprintf
-#undef fputs
-#undef vfprintf
-/* TODO: write */
+#undef isatty
 
 /*
  ANSI codes used by git: m, K
@@ -28,7 +23,10 @@ static HANDLE console;
 static WORD plain_attr;
 static WORD attr;
 static int negative;
-static FILE *last_stream = NULL;
+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__
 typedef struct _CONSOLE_FONT_INFOEX {
@@ -44,27 +42,19 @@ typedef struct _CONSOLE_FONT_INFOEX {
 typedef BOOL (WINAPI *PGETCURRENTCONSOLEFONTEX)(HANDLE, BOOL,
 		PCONSOLE_FONT_INFOEX);
 
-static void print_font_warning(void)
+static void warn_if_raster_font(void)
 {
-	warning("Your console font probably doesn\'t support Unicode. If "
-		"you experience strange characters in the output, consider "
-		"switching to a TrueType font such as Lucida Console!");
-}
-
-static void check_truetype_font(void)
-{
-	static int truetype_font_checked;
 	DWORD fontFamily = 0;
 	PGETCURRENTCONSOLEFONTEX pGetCurrentConsoleFontEx;
 
-	/* don't do this twice */
-	if (truetype_font_checked)
+	/* don't bother if output was ascii only */
+	if (!non_ascii_used)
 		return;
-	truetype_font_checked = 1;
 
 	/* GetCurrentConsoleFontEx is available since Vista */
 	pGetCurrentConsoleFontEx = (PGETCURRENTCONSOLEFONTEX) GetProcAddress(
-			GetModuleHandle("kernel32.dll"), "GetCurrentConsoleFontEx");
+			GetModuleHandle("kernel32.dll"),
+			"GetCurrentConsoleFontEx");
 	if (pGetCurrentConsoleFontEx) {
 		CONSOLE_FONT_INFOEX cfi;
 		cfi.cbSize = sizeof(cfi);
@@ -73,8 +63,8 @@ static void check_truetype_font(void)
 	} else {
 		/* pre-Vista: check default console font in registry */
 		HKEY hkey;
-		if (ERROR_SUCCESS == RegOpenKeyExA(HKEY_CURRENT_USER, "Console", 0,
-				KEY_READ, &hkey)) {
+		if (ERROR_SUCCESS == RegOpenKeyExA(HKEY_CURRENT_USER, "Console",
+				0, KEY_READ, &hkey)) {
 			DWORD size = sizeof(fontFamily);
 			RegQueryValueExA(hkey, "FontFamily", NULL, NULL,
 					(LPVOID) &fontFamily, &size);
@@ -82,61 +72,65 @@ static void check_truetype_font(void)
 		}
 	}
 
-	if (!(fontFamily & TMPF_TRUETYPE))
-		atexit(print_font_warning);
+	if (!(fontFamily & TMPF_TRUETYPE)) {
+		const wchar_t *msg = L"\nWarning: Your console font probably "
+			L"doesn\'t support Unicode. If you experience strange "
+			L"characters in the output, consider switching to a "
+			L"TrueType font such as Lucida Console!\n";
+		DWORD dummy;
+		WriteConsoleW(console, msg, wcslen(msg), &dummy, NULL);
+	}
 }
 
-static int is_console(FILE *stream)
+static int is_console(int fd)
 {
 	CONSOLE_SCREEN_BUFFER_INFO sbi;
 	HANDLE hcon;
 
 	static int initialized = 0;
 
-	/* use cached value if stream hasn't changed */
-	if (stream == last_stream)
-		return console != NULL;
-
-	last_stream = stream;
-	console = NULL;
-
-	/* get OS handle of the stream */
-	hcon = (HANDLE) _get_osfhandle(_fileno(stream));
+	/* get OS handle of the file descriptor */
+	hcon = (HANDLE) _get_osfhandle(fd);
 	if (hcon == INVALID_HANDLE_VALUE)
 		return 0;
 
+	/* check if its a device (i.e. console, printer, serial port) */
+	if (GetFileType(hcon) != FILE_TYPE_CHAR)
+		return 0;
+
 	/* check if its a handle to a console output screen buffer */
 	if (!GetConsoleScreenBufferInfo(hcon, &sbi))
 		return 0;
 
+	/* initialize attributes */
 	if (!initialized) {
 		attr = plain_attr = sbi.wAttributes;
 		negative = 0;
 		initialized = 1;
 	}
 
-	console = hcon;
 	return 1;
 }
 
-static int write_console(const char *str, size_t len)
+#define BUFFER_SIZE 4096
+#define MAX_PARAMS 16
+
+static void write_console(unsigned char *str, size_t len)
 {
-	/* convert utf-8 to utf-16, write directly to console */
-	int wlen = MultiByteToWideChar(CP_UTF8, 0, str, len, NULL, 0);
-	wchar_t *wbuf = (wchar_t *) alloca(wlen * sizeof(wchar_t));
-	MultiByteToWideChar(CP_UTF8, 0, str, len, wbuf, wlen);
+	/* only called from console_thread, so a static buffer will do */
+	static wchar_t wbuf[2 * BUFFER_SIZE + 1];
+	DWORD dummy;
 
-	WriteConsoleW(console, wbuf, wlen, NULL, NULL);
+	/* convert utf-8 to utf-16 */
+	int wlen = MultiByteToWideChar(CP_UTF8, 0, (char*) str, len, wbuf,
+			ARRAY_SIZE(wbuf));
 
-	/*
-	 * if non-ascii characters are printed, check that the current console
-	 * font supports this
-	 */
-	if (wlen != len)
-		check_truetype_font();
+	/* write directly to console */
+	WriteConsoleW(console, wbuf, wlen, &dummy, NULL);
 
-	/* return original (utf-8 encoded) length */
-	return len;
+	/* remember if non-ascii characters are printed */
+	if (wlen != len)
+		non_ascii_used = 1;
 }
 
 #define FOREGROUND_ALL (FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_BLUE)
@@ -182,18 +176,13 @@ static void erase_in_line(void)
 		&dummy);
 }
 
-
-static const char *set_attr(const char *str)
+static void set_attr(char func, const int *params, int paramlen)
 {
-	const char *func;
-	size_t len = strspn(str, "0123456789;");
-	func = str + len;
-
-	switch (*func) {
+	int i;
+	switch (func) {
 	case 'm':
-		do {
-			long val = strtol(str, (char **)&str, 10);
-			switch (val) {
+		for (i = 0; i < paramlen; i++) {
+			switch (params[i]) {
 			case 0: /* reset */
 				attr = plain_attr;
 				negative = 0;
@@ -316,9 +305,7 @@ static const char *set_attr(const char *str)
 				/* Unsupported code */
 				break;
 			}
-			str++;
-		} while (*(str-1) == ';');
-
+		}
 		set_console_attr();
 		break;
 	case 'K':
@@ -328,112 +315,251 @@ static const char *set_attr(const char *str)
 		/* Unsupported code */
 		break;
 	}
-
-	return func + 1;
 }
 
-static int ansi_emulate(const char *str, FILE *stream)
-{
-	int rv = 0;
-	const char *pos = str;
+enum {
+	TEXT = 0, ESCAPE = 033, BRACKET = '['
+};
 
-	fflush(stream);
+static DWORD WINAPI console_thread(LPVOID unused)
+{
+	unsigned char buffer[BUFFER_SIZE];
+	DWORD bytes;
+	int start, end = 0, c, parampos = 0, state = TEXT;
+	int params[MAX_PARAMS];
+
+	while (1) {
+		/* read next chunk of bytes from the pipe */
+		if (!ReadFile(hread, buffer + end, BUFFER_SIZE - end, &bytes,
+				NULL)) {
+			/* exit if pipe has been closed or disconnected */
+			if (GetLastError() == ERROR_PIPE_NOT_CONNECTED ||
+					GetLastError() == ERROR_BROKEN_PIPE)
+				break;
+			/* ignore other errors */
+			continue;
+		}
 
-	while (*pos) {
-		pos = strstr(str, "\033[");
-		if (pos) {
-			size_t len = pos - str;
+		/* scan the bytes and handle ANSI control codes */
+		bytes += end;
+		start = end = 0;
+		while (end < bytes) {
+			c = buffer[end++];
+			switch (state) {
+			case TEXT:
+				if (c == ESCAPE) {
+					/* print text seen so far */
+					if (end - 1 > start)
+						write_console(buffer + start,
+							end - 1 - start);
+
+					/* then start parsing escape sequence */
+					start = end - 1;
+					memset(params, 0, sizeof(params));
+					parampos = 0;
+					state = ESCAPE;
+				}
+				break;
+
+			case ESCAPE:
+				/* continue if "\033[", otherwise bail out */
+				state = (c == BRACKET) ? BRACKET : TEXT;
+				break;
+
+			case BRACKET:
+				/* parse [0-9;]* into array of parameters */
+				if (c >= '0' && c <= '9') {
+					params[parampos] *= 10;
+					params[parampos] += c - '0';
+				} else if (c == ';') {
+					/*
+					 * next parameter, bail out if out of
+					 * bounds
+					 */
+					parampos++;
+					if (parampos >= MAX_PARAMS)
+						state = TEXT;
+				} else {
+					/*
+					 * end of escape sequence, change
+					 * console attributes
+					 */
+					set_attr(c, params, parampos + 1);
+					start = end;
+					state = TEXT;
+				}
+				break;
+			}
+		}
 
-			if (len) {
-				size_t out_len = write_console(str, len);
-				rv += out_len;
-				if (out_len < len)
-					return rv;
+		/* print remaining text unless parsing an escape sequence */
+		if (state == TEXT && end > start) {
+			/* check for incomplete UTF-8 sequences and fix end */
+			if (buffer[end - 1] >= 0x80) {
+				if (buffer[end -1] >= 0xc0)
+					end--;
+				else if (end - 1 > start &&
+						buffer[end - 2] >= 0xe0)
+					end -= 2;
+				else if (end - 2 > start &&
+						buffer[end - 3] >= 0xf0)
+					end -= 3;
 			}
 
-			str = pos + 2;
-			rv += 2;
+			/* print remaining complete UTF-8 sequences */
+			if (end > start)
+				write_console(buffer + start, end - start);
 
-			pos = set_attr(str);
-			rv += pos - str;
-			str = pos;
+			/* move remaining bytes to the front */
+			if (end < bytes)
+				memmove(buffer, buffer + end, bytes - end);
+			end = bytes - end;
 		} else {
-			size_t len = strlen(str);
-			rv += write_console(str, len);
-			return rv;
+			/* all data has been consumed, mark buffer empty */
+			end = 0;
 		}
 	}
-	return rv;
-}
-
-int winansi_fputs(const char *str, FILE *stream)
-{
-	int rv;
-
-	if (!is_console(stream))
-		return fputs(str, stream);
 
-	rv = ansi_emulate(str, stream);
+	/* check if the console font supports unicode */
+	warn_if_raster_font();
 
-	if (rv >= 0)
-		return 0;
-	else
-		return EOF;
+	CloseHandle(hread);
+	return 0;
 }
 
-int winansi_vfprintf(FILE *stream, const char *format, va_list list)
+static void winansi_exit(void)
 {
-	int len, rv;
-	char small_buf[256];
-	char *buf = small_buf;
-	va_list cp;
-
-	if (!is_console(stream))
-		goto abort;
+	/* flush all streams */
+	_flushall();
+
+	/* signal console thread to exit */
+	FlushFileBuffers(hwrite);
+	DisconnectNamedPipe(hwrite);
+
+	/* wait for console thread to copy remaining data */
+	WaitForSingleObject(hthread, INFINITE);
+
+	/* cleanup handles... */
+	if (hwrite1 != INVALID_HANDLE_VALUE)
+		CloseHandle(hwrite1);
+	if (hwrite2 != INVALID_HANDLE_VALUE)
+		CloseHandle(hwrite2);
+	CloseHandle(hwrite);
+	CloseHandle(hthread);
+}
 
-	va_copy(cp, list);
-	len = vsnprintf(small_buf, sizeof(small_buf), format, cp);
-	va_end(cp);
+static void die_lasterr(const char *fmt, ...)
+{
+	va_list params;
+	va_start(params, fmt);
+	errno = err_win_to_posix(GetLastError());
+	die_errno(fmt, params);
+	va_end(params);
+}
 
-	if (len > sizeof(small_buf) - 1) {
-		buf = malloc(len + 1);
-		if (!buf)
-			goto abort;
+static HANDLE duplicate_handle(HANDLE hnd)
+{
+	HANDLE hresult, hproc = GetCurrentProcess();
+	if (!DuplicateHandle(hproc, hnd, hproc, &hresult, 0, TRUE,
+			DUPLICATE_SAME_ACCESS))
+		die_lasterr("DuplicateHandle(%li) failed", (long) hnd);
+	return hresult;
+}
 
-		len = vsnprintf(buf, len + 1, format, list);
-	}
+static HANDLE redirect_console(FILE *stream, HANDLE *phcon, int new_fd)
+{
+	/* get original console handle */
+	int fd = _fileno(stream);
+	HANDLE hcon = (HANDLE) _get_osfhandle(fd);
+	if (hcon == INVALID_HANDLE_VALUE)
+		die_errno("_get_osfhandle(%i) failed", fd);
 
-	rv = ansi_emulate(buf, stream);
+	/* save a copy to phcon and console (used by the background thread) */
+	console = *phcon = duplicate_handle(hcon);
 
-	if (buf != small_buf)
-		free(buf);
-	return rv;
+	/* duplicate new_fd over fd (closes fd and associated handle (hcon)) */
+	if (_dup2(new_fd, fd))
+		die_errno("_dup2(%i, %i) failed", new_fd, fd);
 
-abort:
-	rv = vfprintf(stream, format, list);
-	return rv;
+	/* no buffering, or stdout / stderr will be out of sync */
+	setbuf(stream, NULL);
+	return (HANDLE) _get_osfhandle(fd);
 }
 
-int winansi_fprintf(FILE *stream, const char *format, ...)
+void winansi_init(void)
 {
-	va_list list;
-	int rv;
+	int con1, con2, hwrite_fd;
+	char name[32];
 
-	va_start(list, format);
-	rv = winansi_vfprintf(stream, format, list);
-	va_end(list);
+	/* check if either stdout or stderr is a console output screen buffer */
+	con1 = is_console(1);
+	con2 = is_console(2);
+	if (!con1 && !con2)
+		return;
 
-	return rv;
+	/* create a named pipe to communicate with the console thread */
+	sprintf(name, "\\\\.\\pipe\\winansi%lu", GetCurrentProcessId());
+	hwrite = CreateNamedPipe(name, PIPE_ACCESS_OUTBOUND,
+		PIPE_TYPE_BYTE | PIPE_WAIT, 1, BUFFER_SIZE, 0, 0, NULL);
+	if (hwrite == INVALID_HANDLE_VALUE)
+		die_lasterr("CreateNamedPipe failed");
+
+	hread = CreateFile(name, GENERIC_READ, 0, NULL, OPEN_EXISTING, 0, NULL);
+	if (hread == INVALID_HANDLE_VALUE)
+		die_lasterr("CreateFile for named pipe failed");
+
+	/* start console spool thread on the pipe's read end */
+	hthread = CreateThread(NULL, 0, console_thread, NULL, 0, NULL);
+	if (hthread == INVALID_HANDLE_VALUE)
+		die_lasterr("CreateThread(console_thread) failed");
+
+	/* schedule cleanup routine */
+	if (atexit(winansi_exit))
+		die_errno("atexit(winansi_exit) failed");
+
+	/* create a file descriptor for the write end of the pipe */
+	hwrite_fd = _open_osfhandle((long) duplicate_handle(hwrite), _O_BINARY);
+	if (hwrite_fd == -1)
+		die_errno("_open_osfhandle(%li) failed", (long) hwrite);
+
+	/* redirect stdout / stderr to the pipe */
+	if (con1)
+		hwrite1 = redirect_console(stdout, &hconsole1, hwrite_fd);
+	if (con2)
+		hwrite2 = redirect_console(stderr, &hconsole2, hwrite_fd);
+
+	/* close pipe file descriptor (also closes the duped hwrite) */
+	close(hwrite_fd);
 }
 
-int winansi_printf(const char *format, ...)
+static int is_same_handle(HANDLE hnd, int fd)
 {
-	va_list list;
-	int rv;
+	return hnd != INVALID_HANDLE_VALUE && hnd == (HANDLE) _get_osfhandle(fd);
+}
 
-	va_start(list, format);
-	rv = winansi_vfprintf(stdout, format, list);
-	va_end(list);
+/*
+ * Return true if stdout / stderr is a pipe redirecting to the console.
+ */
+int winansi_isatty(int fd)
+{
+	if (fd == 1 && is_same_handle(hwrite1, 1))
+		return 1;
+	else if (fd == 2 && is_same_handle(hwrite2, 2))
+		return 1;
+	else
+		return isatty(fd);
+}
 
-	return rv;
+/*
+ * Returns the real console handle if stdout / stderr is a pipe redirecting
+ * to the console. Allows spawn / exec to pass the console to the next process.
+ */
+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);
 }
-- 
2.0.0.9635.g0be03cb

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/5] Win32: Thread-safe windows console output
  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
  0 siblings, 1 reply; 22+ messages in thread
From: Peter Krefting @ 2014-06-06 21:29 UTC (permalink / raw)
  To: Stepan Kasal; +Cc: GIT Mailing-list, msysGit, Karsten Blees

Stepan Kasal:

> +	/* only called from console_thread, so a static buffer will do */
> +	static wchar_t wbuf[2 * BUFFER_SIZE + 1];

Wouldn't BUFFER_SIZE + 1 (or even BUFFER_SIZE) do here? If you convert 
from up to BUFFER_SIZE octets of UTF-8 input, you should never get 
back more than BUFFER_SIZE code units of UTF-16 output. Worst case 
would be ASCII, which is one UTF-16 code unit per UTF-8 octet, 
everything else is less (non-BMP is four UTF-8 octets mapping to two 
UTF-16 code units).

-- 
\\// Peter - http://www.softwolves.pp.se/

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/5] Win32: Thread-safe windows console output
  2014-06-06 21:29   ` Peter Krefting
@ 2014-06-06 22:03     ` Karsten Blees
  0 siblings, 0 replies; 22+ messages in thread
From: Karsten Blees @ 2014-06-06 22:03 UTC (permalink / raw)
  To: Peter Krefting, Stepan Kasal; +Cc: GIT Mailing-list, msysGit, Karsten Blees

Am 06.06.2014 23:29, schrieb Peter Krefting:
> Stepan Kasal:
> 
>> +    /* only called from console_thread, so a static buffer will do */
>> +    static wchar_t wbuf[2 * BUFFER_SIZE + 1];
> 
> Wouldn't BUFFER_SIZE + 1 (or even BUFFER_SIZE) do here? If you convert from up to BUFFER_SIZE octets of UTF-8 input, you should never get back more than BUFFER_SIZE code units of UTF-16 output. Worst case would be ASCII, which is one UTF-16 code unit per UTF-8 octet, everything else is less (non-BMP is four UTF-8 octets mapping to two UTF-16 code units).
> 

You're right for MultiByteToWideChar. However, the next patch series will introduce another conversion function that converts invalid UTF-8 to hex code, i.e. two wide chars per invalid UTF-8 char, +1 for L'\0' (see [1] mingw.h:365ff for space requirement rationale). And yet another patch will replace this patch's MultiByteToWideChar for consistentcy [2].

[1] https://github.com/msysgit/git/commit/018c94a8
[2] https://github.com/msysgit/git/commit/45e28a4d

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/5] First part of Unicode console support for msysgit
  2014-06-06 13:42 [PATCH 0/5] First part of Unicode console support for msysgit Stepan Kasal
                   ` (4 preceding siblings ...)
  2014-06-06 13:42 ` [PATCH 5/5] Win32: Thread-safe windows console output Stepan Kasal
@ 2014-06-06 17:44 ` Karsten Blees
  2014-06-06 18:39   ` Stepan Kasal
  2014-06-06 20:48   ` Re: [PATCH 0/5] First part of Unicode console support for msysgit Stepan Kasal
  5 siblings, 2 replies; 22+ messages in thread
From: Karsten Blees @ 2014-06-06 17:44 UTC (permalink / raw)
  To: Stepan Kasal, GIT Mailing-list; +Cc: msysGit

Am 06.06.2014 15:42, schrieb Stepan Kasal:
> Hello,
> 
> this is first part of the unicode support pathes from msysgit.
> 

Nicely done, thanks!

I think its important to reiterate that these patches were written several years apart, so there are some inconsistencies and back-and-forth changes (e.g. [5/5] fixes line break errors introduced in [3/5]). I'm OK with merging this as is, if there are no objections from the list, simply because it represents the battle tested history we have in Git for Windows.

The only real complaint I have is that I'm missing [6/5] "Win32: fix broken pipe detection" [1], which leaves this series in a slightly broken state (terminating the pager will not terminate the calling git process).

Nitpicking follows...

> The first three patches originate in Jun 2010, though some fixups from 2012
> have been squashed in.
> The fourth one is just a trivial prerequisite for
> the last one, that was written in Jan 2012, with a fixup from Mar 2012.
> 

The dates are missing from the patches.
It would also have been nice to name (or link to) the patches you sqashed.

> Regards,
> 	Stepan
> 
> Karsten Blees (5):
>   Support Unicode console output on Windows

This introduces WriteConsoleW, so you could have squashed half of "Win32: fix segfault in WriteConsoleW when debugging in gdb" [2] (second  half in [5/5]).

>   Detect console streams more reliably on Windows
>   Warn if the Windows console font doesn't support Unicode

I think this one includes "MSVC: fix winansi.c compile errors " [3] and "Unicode console: fix font warning on Vista and Win7" [4] (which is partly reverted by [5/5], which also fixes the formatting).

>   Win32: move main macro to a function

Note: this one was submitted seperately on May 29 and May 1 (can't find it in the gmane archive, though).

>   Win32: Thread-safe windows console output
> 
>  compat/mingw.c   |  24 ++-
>  compat/mingw.h   |  24 +--
>  compat/winansi.c | 446 ++++++++++++++++++++++++++++++++++++++++---------------
>  3 files changed, 356 insertions(+), 138 deletions(-)
> 

[1] https://github.com/msysgit/git/commit/67934f93
[2] https://github.com/msysgit/git/commit/cd0792af
[3] https://github.com/msysgit/git/commit/3abcb04d
[4] https://github.com/msysgit/git/commit/981aa538

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Re: [PATCH 0/5] First part of Unicode console support for msysgit
  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-06 20:48   ` Re: [PATCH 0/5] First part of Unicode console support for msysgit Stepan Kasal
  1 sibling, 1 reply; 22+ messages in thread
From: Stepan Kasal @ 2014-06-06 18:39 UTC (permalink / raw)
  To: Karsten Blees; +Cc: GIT Mailing-list, msysGit

Hello Karsten,

On Fri, Jun 06, 2014 at 07:44:33PM +0200, Karsten Blees wrote:
> Nicely done, thanks!

thank you for your kind words.

Please hold back, I will re-submit in a few days.

> Note: this one was submitted seperately on May 29 and May 1 (can't
> find it in the gmane archive, though).

It was cc'ed to msysgit as usual and gmane selected it as the primary
place to store it:
http://thread.gmane.org/gmane.comp.version-control.msysgit/20324

regards,
	Stepan

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 0/6] First part of Unicode console support for msysgit
  2014-06-06 18:39   ` Stepan Kasal
@ 2014-06-07  7:57     ` Stepan Kasal
  2014-06-07  7:57       ` [PATCH v2 1/6] Support Unicode console output on Windows Stepan Kasal
                         ` (5 more replies)
  0 siblings, 6 replies; 22+ messages in thread
From: Stepan Kasal @ 2014-06-07  7:57 UTC (permalink / raw)
  To: GIT Mailing-list; +Cc: msysGit, Stepan Kasal

Hello,
this patch series is to be applied on top of "move main() macro to a function",
discussed in another thread.

I added the two patches Karsten mentioned:
  Win32: add Unicode conversion functions
  Win32: fix broken pipe detection

I also copied the links for the fixups incuded.

Regards,
	Stepan

Karsten Blees (6):
  Support Unicode console output on Windows
  Detect console streams more reliably on Windows
  Warn if the Windows console font doesn't support Unicode
    includes fixups:
    - https://github.com/msysgit/git/commit/3abcb04d
    - https://github.com/msysgit/git/commit/981aa538
  Win32: add Unicode conversion functions
  Win32: Thread-safe windows console output
    includes fixups:
    - https://github.com/msysgit/git/commit/cd0792af
    - https://github.com/msysgit/git/commit/45e28a4d
  Win32: fix broken pipe detection

 compat/mingw.c   |  94 ++++++++++-
 compat/mingw.h   | 112 +++++++++++++-
 compat/winansi.c | 465 +++++++++++++++++++++++++++++++++++++++++--------------
 3 files changed, 546 insertions(+), 125 deletions(-)

-- 
2.0.0.9635.g0be03cb

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 1/6] Support Unicode console output on Windows
  2014-06-07  7:57     ` [PATCH v2 0/6] " Stepan Kasal
@ 2014-06-07  7:57       ` Stepan Kasal
  2014-06-07  7:57       ` [PATCH v2 2/6] Detect console streams more reliably " Stepan Kasal
                         ` (4 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Stepan Kasal @ 2014-06-07  7:57 UTC (permalink / raw)
  To: GIT Mailing-list
  Cc: msysGit, Karsten Blees, Johannes Schindelin, Stepan Kasal

From: Karsten Blees <blees@dcon.de>
Date: Sat, 31 Jul 2010 00:04:01 +0000

WriteConsoleW seems to be the only way to reliably print unicode to the
console (without weird code page conversions).

Also redirects vfprintf to the winansi.c version.

Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Stepan Kasal <kasal@ucw.cz>
---
 compat/mingw.h   |  2 ++
 compat/winansi.c | 26 ++++++++++++++++++++------
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/compat/mingw.h b/compat/mingw.h
index 6dc8b1a..d3cffb7 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -320,9 +320,11 @@ int mingw_raise(int sig);
 int winansi_fputs(const char *str, FILE *stream);
 int winansi_printf(const char *format, ...) __attribute__((format (printf, 1, 2)));
 int winansi_fprintf(FILE *stream, const char *format, ...) __attribute__((format (printf, 2, 3)));
+int winansi_vfprintf(FILE *stream, const char *format, va_list list);
 #define fputs winansi_fputs
 #define printf(...) winansi_printf(__VA_ARGS__)
 #define fprintf(...) winansi_fprintf(__VA_ARGS__)
+#define vfprintf winansi_vfprintf
 
 /*
  * git specific compatibility
diff --git a/compat/winansi.c b/compat/winansi.c
index dedce21..abe0fea 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -3,6 +3,7 @@
  */
 
 #include "../git-compat-util.h"
+#include <malloc.h>
 
 /*
  Functions to be wrapped:
@@ -10,6 +11,7 @@
 #undef printf
 #undef fprintf
 #undef fputs
+#undef vfprintf
 /* TODO: write */
 
 /*
@@ -46,6 +48,18 @@ static void init(void)
 	initialized = 1;
 }
 
+static int write_console(const char *str, size_t len)
+{
+	/* convert utf-8 to utf-16, write directly to console */
+	int wlen = MultiByteToWideChar(CP_UTF8, 0, str, len, NULL, 0);
+	wchar_t *wbuf = (wchar_t *) alloca(wlen * sizeof(wchar_t));
+	MultiByteToWideChar(CP_UTF8, 0, str, len, wbuf, wlen);
+
+	WriteConsoleW(console, wbuf, wlen, NULL, NULL);
+
+	/* return original (utf-8 encoded) length */
+	return len;
+}
 
 #define FOREGROUND_ALL (FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_BLUE)
 #define BACKGROUND_ALL (BACKGROUND_RED | BACKGROUND_GREEN | BACKGROUND_BLUE)
@@ -245,13 +259,15 @@ static int ansi_emulate(const char *str, FILE *stream)
 	int rv = 0;
 	const char *pos = str;
 
+	fflush(stream);
+
 	while (*pos) {
 		pos = strstr(str, "\033[");
 		if (pos) {
 			size_t len = pos - str;
 
 			if (len) {
-				size_t out_len = fwrite(str, 1, len, stream);
+				size_t out_len = write_console(str, len);
 				rv += out_len;
 				if (out_len < len)
 					return rv;
@@ -260,14 +276,12 @@ static int ansi_emulate(const char *str, FILE *stream)
 			str = pos + 2;
 			rv += 2;
 
-			fflush(stream);
-
 			pos = set_attr(str);
 			rv += pos - str;
 			str = pos;
 		} else {
-			rv += strlen(str);
-			fputs(str, stream);
+			size_t len = strlen(str);
+			rv += write_console(str, len);
 			return rv;
 		}
 	}
@@ -294,7 +308,7 @@ int winansi_fputs(const char *str, FILE *stream)
 		return EOF;
 }
 
-static int winansi_vfprintf(FILE *stream, const char *format, va_list list)
+int winansi_vfprintf(FILE *stream, const char *format, va_list list)
 {
 	int len, rv;
 	char small_buf[256];
-- 
2.0.0.9635.g0be03cb

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 2/6] Detect console streams more reliably on Windows
  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       ` Stepan Kasal
  2014-06-07  7:57       ` [PATCH v2 3/6] Warn if the Windows console font doesn't support Unicode Stepan Kasal
                         ` (3 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Stepan Kasal @ 2014-06-07  7:57 UTC (permalink / raw)
  To: GIT Mailing-list
  Cc: msysGit, Karsten Blees, Johannes Schindelin, Stepan Kasal

From: Karsten Blees <blees@dcon.de>
Date: Sat, 31 Jul 2010 00:04:02 +0000

GetStdHandle(STD_OUTPUT_HANDLE) doesn't work for stderr if stdout is
redirected. Use _get_osfhandle of the FILE* instead.

_isatty() is true for all character devices (including parallel and serial
ports). Check return value of GetConsoleScreenBufferInfo instead to
reliably detect console handles (also don't initialize internal state from
an uninitialized CONSOLE_SCREEN_BUFFER_INFO structure if the function
fails).

Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Stepan Kasal <kasal@ucw.cz>
---
 compat/winansi.c | 50 ++++++++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/compat/winansi.c b/compat/winansi.c
index abe0fea..c4be401 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -25,27 +25,39 @@ static HANDLE console;
 static WORD plain_attr;
 static WORD attr;
 static int negative;
+static FILE *last_stream = NULL;
 
-static void init(void)
+static int is_console(FILE *stream)
 {
 	CONSOLE_SCREEN_BUFFER_INFO sbi;
+	HANDLE hcon;
 
 	static int initialized = 0;
-	if (initialized)
-		return;
 
-	console = GetStdHandle(STD_OUTPUT_HANDLE);
-	if (console == INVALID_HANDLE_VALUE)
-		console = NULL;
+	/* use cached value if stream hasn't changed */
+	if (stream == last_stream)
+		return console != NULL;
 
-	if (!console)
-		return;
+	last_stream = stream;
+	console = NULL;
 
-	GetConsoleScreenBufferInfo(console, &sbi);
-	attr = plain_attr = sbi.wAttributes;
-	negative = 0;
+	/* get OS handle of the stream */
+	hcon = (HANDLE) _get_osfhandle(_fileno(stream));
+	if (hcon == INVALID_HANDLE_VALUE)
+		return 0;
+
+	/* check if its a handle to a console output screen buffer */
+	if (!GetConsoleScreenBufferInfo(hcon, &sbi))
+		return 0;
+
+	if (!initialized) {
+		attr = plain_attr = sbi.wAttributes;
+		negative = 0;
+		initialized = 1;
+	}
 
-	initialized = 1;
+	console = hcon;
+	return 1;
 }
 
 static int write_console(const char *str, size_t len)
@@ -292,12 +304,7 @@ int winansi_fputs(const char *str, FILE *stream)
 {
 	int rv;
 
-	if (!isatty(fileno(stream)))
-		return fputs(str, stream);
-
-	init();
-
-	if (!console)
+	if (!is_console(stream))
 		return fputs(str, stream);
 
 	rv = ansi_emulate(str, stream);
@@ -315,12 +322,7 @@ int winansi_vfprintf(FILE *stream, const char *format, va_list list)
 	char *buf = small_buf;
 	va_list cp;
 
-	if (!isatty(fileno(stream)))
-		goto abort;
-
-	init();
-
-	if (!console)
+	if (!is_console(stream))
 		goto abort;
 
 	va_copy(cp, list);
-- 
2.0.0.9635.g0be03cb

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 3/6] Warn if the Windows console font doesn't support Unicode
  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       ` Stepan Kasal
  2014-06-07  7:57       ` [PATCH v2 4/6] Win32: add Unicode conversion functions Stepan Kasal
                         ` (2 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Stepan Kasal @ 2014-06-07  7:57 UTC (permalink / raw)
  To: GIT Mailing-list
  Cc: msysGit, Karsten Blees, Johannes Schindelin, Stepan Kasal

From: Karsten Blees <blees@dcon.de>
Date: Sat, 31 Jul 2010 00:04:03 +0000

Unicode console output won't display correctly with default settings
because the default console font ("Terminal") only supports the system's
OEM charset. Unfortunately, this is a user specific setting, so it cannot
be easily fixed by e.g. some registry tricks in the setup program.

This change prints a warning on exit if console output contained non-ascii
characters and the console font is supposedly not a TrueType font (which
usually have decent Unicode support).

Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Stepan Kasal <kasal@ucw.cz>
---
 compat/winansi.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/compat/winansi.c b/compat/winansi.c
index c4be401..bec6713 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -2,8 +2,11 @@
  * Copyright 2008 Peter Harris <git@peter.is-a-geek.org>
  */
 
+#undef NOGDI
 #include "../git-compat-util.h"
 #include <malloc.h>
+#include <wingdi.h>
+#include <winreg.h>
 
 /*
  Functions to be wrapped:
@@ -27,6 +30,62 @@ static WORD attr;
 static int negative;
 static FILE *last_stream = NULL;
 
+#ifdef __MINGW32__
+typedef struct _CONSOLE_FONT_INFOEX {
+	ULONG cbSize;
+	DWORD nFont;
+	COORD dwFontSize;
+	UINT FontFamily;
+	UINT FontWeight;
+	WCHAR FaceName[LF_FACESIZE];
+} CONSOLE_FONT_INFOEX, *PCONSOLE_FONT_INFOEX;
+#endif
+
+typedef BOOL (WINAPI *PGETCURRENTCONSOLEFONTEX)(HANDLE, BOOL,
+		PCONSOLE_FONT_INFOEX);
+
+static void print_font_warning(void)
+{
+	warning("Your console font probably doesn\'t support Unicode. If "
+		"you experience strange characters in the output, consider "
+		"switching to a TrueType font such as Lucida Console!");
+}
+
+static void check_truetype_font(void)
+{
+	static int truetype_font_checked;
+	DWORD fontFamily = 0;
+	PGETCURRENTCONSOLEFONTEX pGetCurrentConsoleFontEx;
+
+	/* don't do this twice */
+	if (truetype_font_checked)
+		return;
+	truetype_font_checked = 1;
+
+	/* GetCurrentConsoleFontEx is available since Vista */
+	pGetCurrentConsoleFontEx = (PGETCURRENTCONSOLEFONTEX) GetProcAddress(
+			GetModuleHandle("kernel32.dll"), "GetCurrentConsoleFontEx");
+	if (pGetCurrentConsoleFontEx) {
+		CONSOLE_FONT_INFOEX cfi;
+		cfi.cbSize = sizeof(cfi);
+		if (pGetCurrentConsoleFontEx(console, 0, &cfi))
+			fontFamily = cfi.FontFamily;
+	} else {
+		/* pre-Vista: check default console font in registry */
+		HKEY hkey;
+		if (ERROR_SUCCESS == RegOpenKeyExA(HKEY_CURRENT_USER, "Console", 0,
+				KEY_READ, &hkey)) {
+			DWORD size = sizeof(fontFamily);
+			RegQueryValueExA(hkey, "FontFamily", NULL, NULL,
+					(LPVOID) &fontFamily, &size);
+			RegCloseKey(hkey);
+		}
+	}
+
+	if (!(fontFamily & TMPF_TRUETYPE))
+		atexit(print_font_warning);
+}
+
 static int is_console(FILE *stream)
 {
 	CONSOLE_SCREEN_BUFFER_INFO sbi;
@@ -69,6 +128,13 @@ static int write_console(const char *str, size_t len)
 
 	WriteConsoleW(console, wbuf, wlen, NULL, NULL);
 
+	/*
+	 * if non-ascii characters are printed, check that the current console
+	 * font supports this
+	 */
+	if (wlen != len)
+		check_truetype_font();
+
 	/* return original (utf-8 encoded) length */
 	return len;
 }
-- 
2.0.0.9635.g0be03cb

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 4/6] Win32: add Unicode conversion functions
  2014-06-07  7:57     ` [PATCH v2 0/6] " Stepan Kasal
                         ` (2 preceding siblings ...)
  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       ` Stepan Kasal
  2014-06-07  7:57       ` [PATCH v2 5/6] Win32: Thread-safe windows console output Stepan Kasal
  2014-06-07  7:57       ` [PATCH v2 6/6] Win32: fix broken pipe detection Stepan Kasal
  5 siblings, 0 replies; 22+ messages in thread
From: Stepan Kasal @ 2014-06-07  7:57 UTC (permalink / raw)
  To: GIT Mailing-list; +Cc: msysGit, Karsten Blees, Stepan Kasal

From: Karsten Blees <blees@dcon.de>
Date: Fri, 25 Nov 2011 21:05:06 +0100

Add Unicode conversion functions to convert between Windows native UTF-16LE
encoding to UTF-8 and back.

To support repositories with legacy-encoded file names, the UTF-8 to UTF-16
conversion function tries to create valid, unique file names even for
invalid UTF-8 byte sequences, so that these repositories can be checked out
without error.

The current implementation leaves invalid UTF-8 bytes in range 0xa0 - 0xff
as is (producing printable Unicode chars \u00a0 - \u00ff, equivalent to
ISO-8859-1), and converts 0x80 - 0x9f to hex-code (\u0080 - \u009f are
control chars).

The Windows MultiByteToWideChar API was not used as it either drops invalid
UTF-8 sequences (on Win2k/XP; producing non-unique or even empty file
names) or converts them to the replacement char \ufffd (Vista/7; causing
ERROR_INVALID_NAME in subsequent calls to file system APIs).

Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Stepan Kasal <kasal@ucw.cz>
---
 compat/mingw.c |  85 ++++++++++++++++++++++++++++++++++++++++++++++
 compat/mingw.h | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 189 insertions(+)

diff --git a/compat/mingw.c b/compat/mingw.c
index c03bafa..6f1fb10 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1848,6 +1848,91 @@ int mingw_offset_1st_component(const char *path)
 	return offset + is_dir_sep(path[offset]);
 }
 
+int xutftowcsn(wchar_t *wcs, const char *utfs, size_t wcslen, int utflen)
+{
+	int upos = 0, wpos = 0;
+	const unsigned char *utf = (const unsigned char*) utfs;
+	if (!utf || !wcs || wcslen < 1) {
+		errno = EINVAL;
+		return -1;
+	}
+	/* reserve space for \0 */
+	wcslen--;
+	if (utflen < 0)
+		utflen = INT_MAX;
+
+	while (upos < utflen) {
+		int c = utf[upos++] & 0xff;
+		if (utflen == INT_MAX && c == 0)
+			break;
+
+		if (wpos >= wcslen) {
+			wcs[wpos] = 0;
+			errno = ERANGE;
+			return -1;
+		}
+
+		if (c < 0x80) {
+			/* ASCII */
+			wcs[wpos++] = c;
+		} else if (c >= 0xc2 && c < 0xe0 && upos < utflen &&
+				(utf[upos] & 0xc0) == 0x80) {
+			/* 2-byte utf-8 */
+			c = ((c & 0x1f) << 6);
+			c |= (utf[upos++] & 0x3f);
+			wcs[wpos++] = c;
+		} else if (c >= 0xe0 && c < 0xf0 && upos + 1 < utflen &&
+				!(c == 0xe0 && utf[upos] < 0xa0) && /* over-long encoding */
+				(utf[upos] & 0xc0) == 0x80 &&
+				(utf[upos + 1] & 0xc0) == 0x80) {
+			/* 3-byte utf-8 */
+			c = ((c & 0x0f) << 12);
+			c |= ((utf[upos++] & 0x3f) << 6);
+			c |= (utf[upos++] & 0x3f);
+			wcs[wpos++] = c;
+		} else if (c >= 0xf0 && c < 0xf5 && upos + 2 < utflen &&
+				wpos + 1 < wcslen &&
+				!(c == 0xf0 && utf[upos] < 0x90) && /* over-long encoding */
+				!(c == 0xf4 && utf[upos] >= 0x90) && /* > \u10ffff */
+				(utf[upos] & 0xc0) == 0x80 &&
+				(utf[upos + 1] & 0xc0) == 0x80 &&
+				(utf[upos + 2] & 0xc0) == 0x80) {
+			/* 4-byte utf-8: convert to \ud8xx \udcxx surrogate pair */
+			c = ((c & 0x07) << 18);
+			c |= ((utf[upos++] & 0x3f) << 12);
+			c |= ((utf[upos++] & 0x3f) << 6);
+			c |= (utf[upos++] & 0x3f);
+			c -= 0x10000;
+			wcs[wpos++] = 0xd800 | (c >> 10);
+			wcs[wpos++] = 0xdc00 | (c & 0x3ff);
+		} else if (c >= 0xa0) {
+			/* invalid utf-8 byte, printable unicode char: convert 1:1 */
+			wcs[wpos++] = c;
+		} else {
+			/* invalid utf-8 byte, non-printable unicode: convert to hex */
+			static const char *hex = "0123456789abcdef";
+			wcs[wpos++] = hex[c >> 4];
+			if (wpos < wcslen)
+				wcs[wpos++] = hex[c & 0x0f];
+		}
+	}
+	wcs[wpos] = 0;
+	return wpos;
+}
+
+int xwcstoutf(char *utf, const wchar_t *wcs, size_t utflen)
+{
+	if (!wcs || !utf || utflen < 1) {
+		errno = EINVAL;
+		return -1;
+	}
+	utflen = WideCharToMultiByte(CP_UTF8, 0, wcs, -1, utf, utflen, NULL, NULL);
+	if (utflen)
+		return utflen - 1;
+	errno = ERANGE;
+	return -1;
+}
+
 void mingw_startup()
 {
 	/* copy executable name to argv[0] */
diff --git a/compat/mingw.h b/compat/mingw.h
index d3cffb7..921ba08 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -357,6 +357,110 @@ void mingw_open_html(const char *path);
 char **make_augmented_environ(const char *const *vars);
 void free_environ(char **env);
 
+/**
+ * Converts UTF-8 encoded string to UTF-16LE.
+ *
+ * To support repositories with legacy-encoded file names, invalid UTF-8 bytes
+ * 0xa0 - 0xff are converted to corresponding printable Unicode chars \u00a0 -
+ * \u00ff, and invalid UTF-8 bytes 0x80 - 0x9f (which would make non-printable
+ * Unicode) are converted to hex-code.
+ *
+ * Lead-bytes not followed by an appropriate number of trail-bytes, over-long
+ * encodings and 4-byte encodings > \u10ffff are detected as invalid UTF-8.
+ *
+ * Maximum space requirement for the target buffer is two wide chars per UTF-8
+ * char (((strlen(utf) * 2) + 1) [* sizeof(wchar_t)]).
+ *
+ * The maximum space is needed only if the entire input string consists of
+ * invalid UTF-8 bytes in range 0x80-0x9f, as per the following table:
+ *
+ *               |                   | UTF-8 | UTF-16 |
+ *   Code point  |  UTF-8 sequence   | bytes | words  | ratio
+ * --------------+-------------------+-------+--------+-------
+ * 000000-00007f | 0-7f              |   1   |   1    |  1
+ * 000080-0007ff | c2-df + 80-bf     |   2   |   1    |  0.5
+ * 000800-00ffff | e0-ef + 2 * 80-bf |   3   |   1    |  0.33
+ * 010000-10ffff | f0-f4 + 3 * 80-bf |   4   |  2 (a) |  0.5
+ * invalid       | 80-9f             |   1   |  2 (b) |  2
+ * invalid       | a0-ff             |   1   |   1    |  1
+ *
+ * (a) encoded as UTF-16 surrogate pair
+ * (b) encoded as two hex digits
+ *
+ * Note that, while the UTF-8 encoding scheme can be extended to 5-byte, 6-byte
+ * or even indefinite-byte sequences, the largest valid code point \u10ffff
+ * encodes as only 4 UTF-8 bytes.
+ *
+ * Parameters:
+ * wcs: wide char target buffer
+ * utf: string to convert
+ * wcslen: size of target buffer (in wchar_t's)
+ * utflen: size of string to convert, or -1 if 0-terminated
+ *
+ * Returns:
+ * length of converted string (_wcslen(wcs)), or -1 on failure
+ *
+ * Errors:
+ * EINVAL: one of the input parameters is invalid (e.g. NULL)
+ * ERANGE: the output buffer is too small
+ */
+int xutftowcsn(wchar_t *wcs, const char *utf, size_t wcslen, int utflen);
+
+/**
+ * Simplified variant of xutftowcsn, assumes input string is \0-terminated.
+ */
+static inline int xutftowcs(wchar_t *wcs, const char *utf, size_t wcslen)
+{
+	return xutftowcsn(wcs, utf, wcslen, -1);
+}
+
+/**
+ * Simplified file system specific variant of xutftowcsn, assumes output
+ * buffer size is MAX_PATH wide chars and input string is \0-terminated,
+ * fails with ENAMETOOLONG if input string is too long.
+ */
+static inline int xutftowcs_path(wchar_t *wcs, const char *utf)
+{
+	int result = xutftowcsn(wcs, utf, MAX_PATH, -1);
+	if (result < 0 && errno == ERANGE)
+		errno = ENAMETOOLONG;
+	return result;
+}
+
+/**
+ * Converts UTF-16LE encoded string to UTF-8.
+ *
+ * Maximum space requirement for the target buffer is three UTF-8 chars per
+ * wide char ((_wcslen(wcs) * 3) + 1).
+ *
+ * The maximum space is needed only if the entire input string consists of
+ * UTF-16 words in range 0x0800-0xd7ff or 0xe000-0xffff (i.e. \u0800-\uffff
+ * modulo surrogate pairs), as per the following table:
+ *
+ *               |                       | UTF-16 | UTF-8 |
+ *   Code point  |  UTF-16 sequence      | words  | bytes | ratio
+ * --------------+-----------------------+--------+-------+-------
+ * 000000-00007f | 0000-007f             |   1    |   1   |  1
+ * 000080-0007ff | 0080-07ff             |   1    |   2   |  2
+ * 000800-00ffff | 0800-d7ff / e000-ffff |   1    |   3   |  3
+ * 010000-10ffff | d800-dbff + dc00-dfff |   2    |   4   |  2
+ *
+ * Note that invalid code points > 10ffff cannot be represented in UTF-16.
+ *
+ * Parameters:
+ * utf: target buffer
+ * wcs: wide string to convert
+ * utflen: size of target buffer
+ *
+ * Returns:
+ * length of converted string, or -1 on failure
+ *
+ * Errors:
+ * EINVAL: one of the input parameters is invalid (e.g. NULL)
+ * ERANGE: the output buffer is too small
+ */
+int xwcstoutf(char *utf, const wchar_t *wcs, size_t utflen);
+
 /*
  * A critical section used in the implementation of the spawn
  * functions (mingw_spawnv[p]e()) and waitpid(). Intialised in
-- 
2.0.0.9635.g0be03cb

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 5/6] Win32: Thread-safe windows console output
  2014-06-07  7:57     ` [PATCH v2 0/6] " Stepan Kasal
                         ` (3 preceding siblings ...)
  2014-06-07  7:57       ` [PATCH v2 4/6] Win32: add Unicode conversion functions Stepan Kasal
@ 2014-06-07  7:57       ` Stepan Kasal
  2014-06-13  6:10         ` Johannes Sixt
  2014-06-07  7:57       ` [PATCH v2 6/6] Win32: fix broken pipe detection Stepan Kasal
  5 siblings, 1 reply; 22+ messages in thread
From: Stepan Kasal @ 2014-06-07  7:57 UTC (permalink / raw)
  To: GIT Mailing-list; +Cc: msysGit, Karsten Blees, Stepan Kasal

From: Karsten Blees <blees@dcon.de>
Date: Sat, 14 Jan 2012 22:24:19 +0100

Winansi.c has many static variables that are accessed and modified from
the [v][f]printf / fputs functions overridden in the file. This may cause
multi threaded git commands that print to the console to produce corrupted
output or even crash.

Additionally, winansi.c doesn't override all functions that can be used to
print to the console (e.g. fwrite, write, fputc are missing), so that ANSI
escapes don't work properly for some git commands (e.g. git-grep).

Instead of doing ANSI emulation in just a few wrapped functions on top of
the IO API, let's plug into the IO system and take advantage of the thread
safety inherent to the IO system.

Redirect stdout and stderr to a pipe if they point to the console. A
background thread reads from the pipe, handles ANSI escape sequences and
UTF-8 to UTF-16 conversion, then writes to the console.

The pipe-based stdout and stderr replacements must be set to unbuffered, as
MSVCRT doesn't support line buffering and fully buffered streams are
inappropriate for console output.

Due to the byte-oriented pipe, ANSI escape sequences and multi-byte UTF-8
sequences can no longer be expected to arrive in one piece. Replace the
string-based ansi_emulate() with a simple stateful parser (this also fixes
colored diff hunk headers, which were broken as of commit 2efcc977).

Override isatty to return true for the pipes redirecting to the console.

Exec/spawn obtain the original console handle to pass to the next process
via winansi_get_osfhandle().

All other overrides are gone, the default stdio implementations work as
expected with the piped stdout/stderr descriptors.

Global variables are either initialized on startup (single threaded) or
exclusively modified by the background thread. Threads communicate through
the pipe, no further synchronization is necessary.

The background thread is terminated by disonnecting the pipe after flushing
the stdio and pipe buffers. This doesn't work for anonymous pipes (created
via CreatePipe), as DisconnectNamedPipe only works on the read end, which
discards remaining data. Thus we have to setup the pipe manually, with the
write end beeing the server (opened with CreateNamedPipe) and the read end
the client (opened with CreateFile).

Limitations: doesn't track reopened or duped file descriptors, i.e.:
- fdopen(1/2) returns fully buffered streams
- dup(1/2), dup2(1/2) returns normal pipe descriptors (i.e. isatty() =
  false, winansi_get_osfhandle won't return the original console handle)

Currently, only the git-format-patch command uses xfdopen(xdup(1)) (see
"realstdout" in builtin/log.c), but works well with these limitations.

Many thanks to Atsushi Nakagawa <atnak@chejz.com> for suggesting and
reviewing the thread-exit-mechanism.

Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Stepan Kasal <kasal@ucw.cz>
---
 compat/mingw.c   |   9 +-
 compat/mingw.h   |  12 +-
 compat/winansi.c | 401 ++++++++++++++++++++++++++++++++++++-------------------
 3 files changed, 273 insertions(+), 149 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 6f1fb10..d242557 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -865,9 +865,9 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **env,
 	memset(&si, 0, sizeof(si));
 	si.cb = sizeof(si);
 	si.dwFlags = STARTF_USESTDHANDLES;
-	si.hStdInput = (HANDLE) _get_osfhandle(fhin);
-	si.hStdOutput = (HANDLE) _get_osfhandle(fhout);
-	si.hStdError = (HANDLE) _get_osfhandle(fherr);
+	si.hStdInput = winansi_get_osfhandle(fhin);
+	si.hStdOutput = winansi_get_osfhandle(fhout);
+	si.hStdError = winansi_get_osfhandle(fherr);
 
 	/* concatenate argv, quoting args as we go */
 	strbuf_init(&args, 0);
@@ -1946,4 +1946,7 @@ void mingw_startup()
 	_setmode(_fileno(stdin), _O_BINARY);
 	_setmode(_fileno(stdout), _O_BINARY);
 	_setmode(_fileno(stderr), _O_BINARY);
+
+	/* initialize Unicode console */
+	winansi_init();
 }
diff --git a/compat/mingw.h b/compat/mingw.h
index 921ba08..4b638d8 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -317,14 +317,10 @@ int mingw_raise(int sig);
  * ANSI emulation wrappers
  */
 
-int winansi_fputs(const char *str, FILE *stream);
-int winansi_printf(const char *format, ...) __attribute__((format (printf, 1, 2)));
-int winansi_fprintf(FILE *stream, const char *format, ...) __attribute__((format (printf, 2, 3)));
-int winansi_vfprintf(FILE *stream, const char *format, va_list list);
-#define fputs winansi_fputs
-#define printf(...) winansi_printf(__VA_ARGS__)
-#define fprintf(...) winansi_fprintf(__VA_ARGS__)
-#define vfprintf winansi_vfprintf
+void winansi_init(void);
+int winansi_isatty(int fd);
+HANDLE winansi_get_osfhandle(int fd);
+#define isatty winansi_isatty
 
 /*
  * git specific compatibility
diff --git a/compat/winansi.c b/compat/winansi.c
index bec6713..fcdd6dc 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -4,18 +4,13 @@
 
 #undef NOGDI
 #include "../git-compat-util.h"
-#include <malloc.h>
 #include <wingdi.h>
 #include <winreg.h>
 
 /*
  Functions to be wrapped:
 */
-#undef printf
-#undef fprintf
-#undef fputs
-#undef vfprintf
-/* TODO: write */
+#undef isatty
 
 /*
  ANSI codes used by git: m, K
@@ -28,7 +23,10 @@ static HANDLE console;
 static WORD plain_attr;
 static WORD attr;
 static int negative;
-static FILE *last_stream = NULL;
+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__
 typedef struct _CONSOLE_FONT_INFOEX {
@@ -44,27 +42,19 @@ typedef struct _CONSOLE_FONT_INFOEX {
 typedef BOOL (WINAPI *PGETCURRENTCONSOLEFONTEX)(HANDLE, BOOL,
 		PCONSOLE_FONT_INFOEX);
 
-static void print_font_warning(void)
+static void warn_if_raster_font(void)
 {
-	warning("Your console font probably doesn\'t support Unicode. If "
-		"you experience strange characters in the output, consider "
-		"switching to a TrueType font such as Lucida Console!");
-}
-
-static void check_truetype_font(void)
-{
-	static int truetype_font_checked;
 	DWORD fontFamily = 0;
 	PGETCURRENTCONSOLEFONTEX pGetCurrentConsoleFontEx;
 
-	/* don't do this twice */
-	if (truetype_font_checked)
+	/* don't bother if output was ascii only */
+	if (!non_ascii_used)
 		return;
-	truetype_font_checked = 1;
 
 	/* GetCurrentConsoleFontEx is available since Vista */
 	pGetCurrentConsoleFontEx = (PGETCURRENTCONSOLEFONTEX) GetProcAddress(
-			GetModuleHandle("kernel32.dll"), "GetCurrentConsoleFontEx");
+			GetModuleHandle("kernel32.dll"),
+			"GetCurrentConsoleFontEx");
 	if (pGetCurrentConsoleFontEx) {
 		CONSOLE_FONT_INFOEX cfi;
 		cfi.cbSize = sizeof(cfi);
@@ -73,8 +63,8 @@ static void check_truetype_font(void)
 	} else {
 		/* pre-Vista: check default console font in registry */
 		HKEY hkey;
-		if (ERROR_SUCCESS == RegOpenKeyExA(HKEY_CURRENT_USER, "Console", 0,
-				KEY_READ, &hkey)) {
+		if (ERROR_SUCCESS == RegOpenKeyExA(HKEY_CURRENT_USER, "Console",
+				0, KEY_READ, &hkey)) {
 			DWORD size = sizeof(fontFamily);
 			RegQueryValueExA(hkey, "FontFamily", NULL, NULL,
 					(LPVOID) &fontFamily, &size);
@@ -82,61 +72,64 @@ static void check_truetype_font(void)
 		}
 	}
 
-	if (!(fontFamily & TMPF_TRUETYPE))
-		atexit(print_font_warning);
+	if (!(fontFamily & TMPF_TRUETYPE)) {
+		const wchar_t *msg = L"\nWarning: Your console font probably "
+			L"doesn\'t support Unicode. If you experience strange "
+			L"characters in the output, consider switching to a "
+			L"TrueType font such as Consolas!\n";
+		DWORD dummy;
+		WriteConsoleW(console, msg, wcslen(msg), &dummy, NULL);
+	}
 }
 
-static int is_console(FILE *stream)
+static int is_console(int fd)
 {
 	CONSOLE_SCREEN_BUFFER_INFO sbi;
 	HANDLE hcon;
 
 	static int initialized = 0;
 
-	/* use cached value if stream hasn't changed */
-	if (stream == last_stream)
-		return console != NULL;
-
-	last_stream = stream;
-	console = NULL;
-
-	/* get OS handle of the stream */
-	hcon = (HANDLE) _get_osfhandle(_fileno(stream));
+	/* get OS handle of the file descriptor */
+	hcon = (HANDLE) _get_osfhandle(fd);
 	if (hcon == INVALID_HANDLE_VALUE)
 		return 0;
 
+	/* check if its a device (i.e. console, printer, serial port) */
+	if (GetFileType(hcon) != FILE_TYPE_CHAR)
+		return 0;
+
 	/* check if its a handle to a console output screen buffer */
 	if (!GetConsoleScreenBufferInfo(hcon, &sbi))
 		return 0;
 
+	/* initialize attributes */
 	if (!initialized) {
 		attr = plain_attr = sbi.wAttributes;
 		negative = 0;
 		initialized = 1;
 	}
 
-	console = hcon;
 	return 1;
 }
 
-static int write_console(const char *str, size_t len)
+#define BUFFER_SIZE 4096
+#define MAX_PARAMS 16
+
+static void write_console(unsigned char *str, size_t len)
 {
-	/* convert utf-8 to utf-16, write directly to console */
-	int wlen = MultiByteToWideChar(CP_UTF8, 0, str, len, NULL, 0);
-	wchar_t *wbuf = (wchar_t *) alloca(wlen * sizeof(wchar_t));
-	MultiByteToWideChar(CP_UTF8, 0, str, len, wbuf, wlen);
+	/* only called from console_thread, so a static buffer will do */
+	static wchar_t wbuf[2 * BUFFER_SIZE + 1];
+	DWORD dummy;
 
-	WriteConsoleW(console, wbuf, wlen, NULL, NULL);
+	/* convert utf-8 to utf-16 */
+	int wlen = xutftowcsn(wbuf, (char*) str, ARRAY_SIZE(wbuf), len);
 
-	/*
-	 * if non-ascii characters are printed, check that the current console
-	 * font supports this
-	 */
-	if (wlen != len)
-		check_truetype_font();
+	/* write directly to console */
+	WriteConsoleW(console, wbuf, wlen, &dummy, NULL);
 
-	/* return original (utf-8 encoded) length */
-	return len;
+	/* remember if non-ascii characters are printed */
+	if (wlen != len)
+		non_ascii_used = 1;
 }
 
 #define FOREGROUND_ALL (FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_BLUE)
@@ -182,18 +175,13 @@ static void erase_in_line(void)
 		&dummy);
 }
 
-
-static const char *set_attr(const char *str)
+static void set_attr(char func, const int *params, int paramlen)
 {
-	const char *func;
-	size_t len = strspn(str, "0123456789;");
-	func = str + len;
-
-	switch (*func) {
+	int i;
+	switch (func) {
 	case 'm':
-		do {
-			long val = strtol(str, (char **)&str, 10);
-			switch (val) {
+		for (i = 0; i < paramlen; i++) {
+			switch (params[i]) {
 			case 0: /* reset */
 				attr = plain_attr;
 				negative = 0;
@@ -316,9 +304,7 @@ static const char *set_attr(const char *str)
 				/* Unsupported code */
 				break;
 			}
-			str++;
-		} while (*(str-1) == ';');
-
+		}
 		set_console_attr();
 		break;
 	case 'K':
@@ -328,112 +314,251 @@ static const char *set_attr(const char *str)
 		/* Unsupported code */
 		break;
 	}
-
-	return func + 1;
 }
 
-static int ansi_emulate(const char *str, FILE *stream)
-{
-	int rv = 0;
-	const char *pos = str;
+enum {
+	TEXT = 0, ESCAPE = 033, BRACKET = '['
+};
 
-	fflush(stream);
+static DWORD WINAPI console_thread(LPVOID unused)
+{
+	unsigned char buffer[BUFFER_SIZE];
+	DWORD bytes;
+	int start, end = 0, c, parampos = 0, state = TEXT;
+	int params[MAX_PARAMS];
+
+	while (1) {
+		/* read next chunk of bytes from the pipe */
+		if (!ReadFile(hread, buffer + end, BUFFER_SIZE - end, &bytes,
+				NULL)) {
+			/* exit if pipe has been closed or disconnected */
+			if (GetLastError() == ERROR_PIPE_NOT_CONNECTED ||
+					GetLastError() == ERROR_BROKEN_PIPE)
+				break;
+			/* ignore other errors */
+			continue;
+		}
 
-	while (*pos) {
-		pos = strstr(str, "\033[");
-		if (pos) {
-			size_t len = pos - str;
+		/* scan the bytes and handle ANSI control codes */
+		bytes += end;
+		start = end = 0;
+		while (end < bytes) {
+			c = buffer[end++];
+			switch (state) {
+			case TEXT:
+				if (c == ESCAPE) {
+					/* print text seen so far */
+					if (end - 1 > start)
+						write_console(buffer + start,
+							end - 1 - start);
+
+					/* then start parsing escape sequence */
+					start = end - 1;
+					memset(params, 0, sizeof(params));
+					parampos = 0;
+					state = ESCAPE;
+				}
+				break;
+
+			case ESCAPE:
+				/* continue if "\033[", otherwise bail out */
+				state = (c == BRACKET) ? BRACKET : TEXT;
+				break;
+
+			case BRACKET:
+				/* parse [0-9;]* into array of parameters */
+				if (c >= '0' && c <= '9') {
+					params[parampos] *= 10;
+					params[parampos] += c - '0';
+				} else if (c == ';') {
+					/*
+					 * next parameter, bail out if out of
+					 * bounds
+					 */
+					parampos++;
+					if (parampos >= MAX_PARAMS)
+						state = TEXT;
+				} else {
+					/*
+					 * end of escape sequence, change
+					 * console attributes
+					 */
+					set_attr(c, params, parampos + 1);
+					start = end;
+					state = TEXT;
+				}
+				break;
+			}
+		}
 
-			if (len) {
-				size_t out_len = write_console(str, len);
-				rv += out_len;
-				if (out_len < len)
-					return rv;
+		/* print remaining text unless parsing an escape sequence */
+		if (state == TEXT && end > start) {
+			/* check for incomplete UTF-8 sequences and fix end */
+			if (buffer[end - 1] >= 0x80) {
+				if (buffer[end -1] >= 0xc0)
+					end--;
+				else if (end - 1 > start &&
+						buffer[end - 2] >= 0xe0)
+					end -= 2;
+				else if (end - 2 > start &&
+						buffer[end - 3] >= 0xf0)
+					end -= 3;
 			}
 
-			str = pos + 2;
-			rv += 2;
+			/* print remaining complete UTF-8 sequences */
+			if (end > start)
+				write_console(buffer + start, end - start);
 
-			pos = set_attr(str);
-			rv += pos - str;
-			str = pos;
+			/* move remaining bytes to the front */
+			if (end < bytes)
+				memmove(buffer, buffer + end, bytes - end);
+			end = bytes - end;
 		} else {
-			size_t len = strlen(str);
-			rv += write_console(str, len);
-			return rv;
+			/* all data has been consumed, mark buffer empty */
+			end = 0;
 		}
 	}
-	return rv;
-}
-
-int winansi_fputs(const char *str, FILE *stream)
-{
-	int rv;
-
-	if (!is_console(stream))
-		return fputs(str, stream);
 
-	rv = ansi_emulate(str, stream);
+	/* check if the console font supports unicode */
+	warn_if_raster_font();
 
-	if (rv >= 0)
-		return 0;
-	else
-		return EOF;
+	CloseHandle(hread);
+	return 0;
 }
 
-int winansi_vfprintf(FILE *stream, const char *format, va_list list)
+static void winansi_exit(void)
 {
-	int len, rv;
-	char small_buf[256];
-	char *buf = small_buf;
-	va_list cp;
-
-	if (!is_console(stream))
-		goto abort;
+	/* flush all streams */
+	_flushall();
+
+	/* signal console thread to exit */
+	FlushFileBuffers(hwrite);
+	DisconnectNamedPipe(hwrite);
+
+	/* wait for console thread to copy remaining data */
+	WaitForSingleObject(hthread, INFINITE);
+
+	/* cleanup handles... */
+	if (hwrite1 != INVALID_HANDLE_VALUE)
+		CloseHandle(hwrite1);
+	if (hwrite2 != INVALID_HANDLE_VALUE)
+		CloseHandle(hwrite2);
+	CloseHandle(hwrite);
+	CloseHandle(hthread);
+}
 
-	va_copy(cp, list);
-	len = vsnprintf(small_buf, sizeof(small_buf), format, cp);
-	va_end(cp);
+static void die_lasterr(const char *fmt, ...)
+{
+	va_list params;
+	va_start(params, fmt);
+	errno = err_win_to_posix(GetLastError());
+	die_errno(fmt, params);
+	va_end(params);
+}
 
-	if (len > sizeof(small_buf) - 1) {
-		buf = malloc(len + 1);
-		if (!buf)
-			goto abort;
+static HANDLE duplicate_handle(HANDLE hnd)
+{
+	HANDLE hresult, hproc = GetCurrentProcess();
+	if (!DuplicateHandle(hproc, hnd, hproc, &hresult, 0, TRUE,
+			DUPLICATE_SAME_ACCESS))
+		die_lasterr("DuplicateHandle(%li) failed", (long) hnd);
+	return hresult;
+}
 
-		len = vsnprintf(buf, len + 1, format, list);
-	}
+static HANDLE redirect_console(FILE *stream, HANDLE *phcon, int new_fd)
+{
+	/* get original console handle */
+	int fd = _fileno(stream);
+	HANDLE hcon = (HANDLE) _get_osfhandle(fd);
+	if (hcon == INVALID_HANDLE_VALUE)
+		die_errno("_get_osfhandle(%i) failed", fd);
 
-	rv = ansi_emulate(buf, stream);
+	/* save a copy to phcon and console (used by the background thread) */
+	console = *phcon = duplicate_handle(hcon);
 
-	if (buf != small_buf)
-		free(buf);
-	return rv;
+	/* duplicate new_fd over fd (closes fd and associated handle (hcon)) */
+	if (_dup2(new_fd, fd))
+		die_errno("_dup2(%i, %i) failed", new_fd, fd);
 
-abort:
-	rv = vfprintf(stream, format, list);
-	return rv;
+	/* no buffering, or stdout / stderr will be out of sync */
+	setbuf(stream, NULL);
+	return (HANDLE) _get_osfhandle(fd);
 }
 
-int winansi_fprintf(FILE *stream, const char *format, ...)
+void winansi_init(void)
 {
-	va_list list;
-	int rv;
+	int con1, con2, hwrite_fd;
+	char name[32];
 
-	va_start(list, format);
-	rv = winansi_vfprintf(stream, format, list);
-	va_end(list);
+	/* check if either stdout or stderr is a console output screen buffer */
+	con1 = is_console(1);
+	con2 = is_console(2);
+	if (!con1 && !con2)
+		return;
 
-	return rv;
+	/* create a named pipe to communicate with the console thread */
+	sprintf(name, "\\\\.\\pipe\\winansi%lu", GetCurrentProcessId());
+	hwrite = CreateNamedPipe(name, PIPE_ACCESS_OUTBOUND,
+		PIPE_TYPE_BYTE | PIPE_WAIT, 1, BUFFER_SIZE, 0, 0, NULL);
+	if (hwrite == INVALID_HANDLE_VALUE)
+		die_lasterr("CreateNamedPipe failed");
+
+	hread = CreateFile(name, GENERIC_READ, 0, NULL, OPEN_EXISTING, 0, NULL);
+	if (hread == INVALID_HANDLE_VALUE)
+		die_lasterr("CreateFile for named pipe failed");
+
+	/* start console spool thread on the pipe's read end */
+	hthread = CreateThread(NULL, 0, console_thread, NULL, 0, NULL);
+	if (hthread == INVALID_HANDLE_VALUE)
+		die_lasterr("CreateThread(console_thread) failed");
+
+	/* schedule cleanup routine */
+	if (atexit(winansi_exit))
+		die_errno("atexit(winansi_exit) failed");
+
+	/* create a file descriptor for the write end of the pipe */
+	hwrite_fd = _open_osfhandle((long) duplicate_handle(hwrite), _O_BINARY);
+	if (hwrite_fd == -1)
+		die_errno("_open_osfhandle(%li) failed", (long) hwrite);
+
+	/* redirect stdout / stderr to the pipe */
+	if (con1)
+		hwrite1 = redirect_console(stdout, &hconsole1, hwrite_fd);
+	if (con2)
+		hwrite2 = redirect_console(stderr, &hconsole2, hwrite_fd);
+
+	/* close pipe file descriptor (also closes the duped hwrite) */
+	close(hwrite_fd);
 }
 
-int winansi_printf(const char *format, ...)
+static int is_same_handle(HANDLE hnd, int fd)
 {
-	va_list list;
-	int rv;
+	return hnd != INVALID_HANDLE_VALUE && hnd == (HANDLE) _get_osfhandle(fd);
+}
 
-	va_start(list, format);
-	rv = winansi_vfprintf(stdout, format, list);
-	va_end(list);
+/*
+ * Return true if stdout / stderr is a pipe redirecting to the console.
+ */
+int winansi_isatty(int fd)
+{
+	if (fd == 1 && is_same_handle(hwrite1, 1))
+		return 1;
+	else if (fd == 2 && is_same_handle(hwrite2, 2))
+		return 1;
+	else
+		return isatty(fd);
+}
 
-	return rv;
+/*
+ * Returns the real console handle if stdout / stderr is a pipe redirecting
+ * to the console. Allows spawn / exec to pass the console to the next process.
+ */
+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);
 }
-- 
2.0.0.9635.g0be03cb

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 5/6] Win32: Thread-safe windows console output
  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           ` [PATCH 7/6] Win32: reliably detect console pipe handles Karsten Blees
  0 siblings, 1 reply; 22+ messages in thread
From: Johannes Sixt @ 2014-06-13  6:10 UTC (permalink / raw)
  To: Stepan Kasal; +Cc: GIT Mailing-list, msysGit, Karsten Blees

Am 07.06.2014 09:57, schrieb Stepan Kasal:
> From: Karsten Blees <blees@dcon.de>
> Date: Sat, 14 Jan 2012 22:24:19 +0100
>
> Winansi.c has many static variables that are accessed and modified from
> the [v][f]printf / fputs functions overridden in the file. This may cause
> multi threaded git commands that print to the console to produce corrupted
> output or even crash.
>
> Additionally, winansi.c doesn't override all functions that can be used to
> print to the console (e.g. fwrite, write, fputc are missing), so that ANSI
> escapes don't work properly for some git commands (e.g. git-grep).
>
> Instead of doing ANSI emulation in just a few wrapped functions on top of
> the IO API, let's plug into the IO system and take advantage of the thread
> safety inherent to the IO system.
>
> Redirect stdout and stderr to a pipe if they point to the console. A
> background thread reads from the pipe, handles ANSI escape sequences and
> UTF-8 to UTF-16 conversion, then writes to the console.

There's something fishy with this patch. Please checkout and build 
eac14f8909d9. Then run t5000-tar-tree.sh like so from a CMD prompt:

   sh t5000-tar-tree.sh -v -i

Notice that in test 36 (invoke tar filter by extension) the tar file is 
written to the console instead of the file. Hit Ctrl-C to interrupt the 
test; do not remove the trash directory. You can verify the incorrect 
behavior like this:

   cd "t\trash directory.t5000-tar-tree"
   ..\..\git archive -o config-implicit.tar.foo HEAD

It writes the tar file to the console. When you build the parent commit, 
and repeat these two commands, there is no unexpected console output.

The patch fcd428f4a952 (Win32: fix broken pipe detection) does not fix the 
incorrect behavior.

I haven't dug (and won't dig) deeper than that.

-- Hannes

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 7/6] Win32: reliably detect console pipe handles
  2014-06-13  6:10         ` Johannes Sixt
@ 2014-06-13 22:09           ` Karsten Blees
  0 siblings, 0 replies; 22+ messages in thread
From: Karsten Blees @ 2014-06-13 22:09 UTC (permalink / raw)
  To: Johannes Sixt, Stepan Kasal; +Cc: GIT Mailing-list, msysGit, Karsten Blees

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 6/6] Win32: fix broken pipe detection
  2014-06-07  7:57     ` [PATCH v2 0/6] " Stepan Kasal
                         ` (4 preceding siblings ...)
  2014-06-07  7:57       ` [PATCH v2 5/6] Win32: Thread-safe windows console output Stepan Kasal
@ 2014-06-07  7:57       ` Stepan Kasal
  5 siblings, 0 replies; 22+ messages in thread
From: Stepan Kasal @ 2014-06-07  7:57 UTC (permalink / raw)
  To: GIT Mailing-list; +Cc: msysGit, Karsten Blees, Stepan Kasal

From: Karsten Blees <blees@dcon.de>
Date: Thu, 1 Mar 2012 21:53:54 +0100

As of "Win32: Thread-safe windows console output", git-log no longer
terminates when the pager process dies. This is due to disabling buffering
for the replaced stdout / stderr streams. Git-log will periodically fflush
stdout (see write_or_die.c/mayble_flush_or_die()), but with no buffering,
this is a NOP that always succeeds (so we never detect the EPIPE error).

Exchange the original console handles with our console thread pipe handles
by accessing the internal MSVCRT data structures directly (which are
exposed via __pioinfo for some reason).

Implement this with minimal assumptions about the actual data structure to
make it work with different (hopefully even future) MSVCRT versions.

While messing with internal data structures is ugly, this patch solves the
problem at the source instead of adding more workarounds. We no longer need
the special winansi_isatty override, and the limitations documented in
"Win32: Thread-safe windows console output" are gone (i.e. fdopen(1/2)
returns unbuffered streams now, and isatty() for duped console file
descriptors works as expected).

Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Stepan Kasal <kasal@ucw.cz>
---
 compat/mingw.h   |   2 -
 compat/winansi.c | 114 ++++++++++++++++++++++++++++++++++---------------------
 2 files changed, 70 insertions(+), 46 deletions(-)

diff --git a/compat/mingw.h b/compat/mingw.h
index 4b638d8..8dac6f9 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -318,9 +318,7 @@ int mingw_raise(int sig);
  */
 
 void winansi_init(void);
-int winansi_isatty(int fd);
 HANDLE winansi_get_osfhandle(int fd);
-#define isatty winansi_isatty
 
 /*
  * git specific compatibility
diff --git a/compat/winansi.c b/compat/winansi.c
index fcdd6dc..f96d5c2 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -8,11 +8,6 @@
 #include <winreg.h>
 
 /*
- Functions to be wrapped:
-*/
-#undef isatty
-
-/*
  ANSI codes used by git: m, K
 
  This file is git-specific. Therefore, this file does not attempt
@@ -104,6 +99,7 @@ static int is_console(int fd)
 
 	/* initialize attributes */
 	if (!initialized) {
+		console = hcon;
 		attr = plain_attr = sbi.wAttributes;
 		negative = 0;
 		initialized = 1;
@@ -465,29 +461,80 @@ static HANDLE duplicate_handle(HANDLE hnd)
 	return hresult;
 }
 
-static HANDLE redirect_console(FILE *stream, HANDLE *phcon, int new_fd)
-{
-	/* get original console handle */
-	int fd = _fileno(stream);
-	HANDLE hcon = (HANDLE) _get_osfhandle(fd);
-	if (hcon == INVALID_HANDLE_VALUE)
-		die_errno("_get_osfhandle(%i) failed", fd);
 
-	/* save a copy to phcon and console (used by the background thread) */
-	console = *phcon = duplicate_handle(hcon);
+/*
+ * Make MSVCRT's internal file descriptor control structure accessible
+ * so that we can tweak OS handles and flags directly (we need MSVCRT
+ * to treat our pipe handle as if it were a console).
+ *
+ * We assume that the ioinfo structure (exposed by MSVCRT.dll via
+ * __pioinfo) starts with the OS handle and the flags. The exact size
+ * varies between MSVCRT versions, so we try different sizes until
+ * toggling the FDEV bit of _pioinfo(1)->osflags is reflected in
+ * isatty(1).
+ */
+typedef struct {
+	HANDLE osfhnd;
+	char osflags;
+} ioinfo;
+
+extern __declspec(dllimport) ioinfo *__pioinfo[];
 
-	/* duplicate new_fd over fd (closes fd and associated handle (hcon)) */
-	if (_dup2(new_fd, fd))
-		die_errno("_dup2(%i, %i) failed", new_fd, fd);
+static size_t sizeof_ioinfo = 0;
 
-	/* no buffering, or stdout / stderr will be out of sync */
-	setbuf(stream, NULL);
-	return (HANDLE) _get_osfhandle(fd);
+#define IOINFO_L2E 5
+#define IOINFO_ARRAY_ELTS (1 << IOINFO_L2E)
+
+#define FDEV  0x40
+
+static inline ioinfo* _pioinfo(int fd)
+{
+	return (ioinfo*)((char*)__pioinfo[fd >> IOINFO_L2E] +
+			(fd & (IOINFO_ARRAY_ELTS - 1)) * sizeof_ioinfo);
+}
+
+static int init_sizeof_ioinfo()
+{
+	int istty, wastty;
+	/* don't init twice */
+	if (sizeof_ioinfo)
+		return sizeof_ioinfo >= 256;
+
+	sizeof_ioinfo = sizeof(ioinfo);
+	wastty = isatty(1);
+	while (sizeof_ioinfo < 256) {
+		/* toggle FDEV flag, check isatty, then toggle back */
+		_pioinfo(1)->osflags ^= FDEV;
+		istty = isatty(1);
+		_pioinfo(1)->osflags ^= FDEV;
+		/* return if we found the correct size */
+		if (istty != wastty)
+			return 0;
+		sizeof_ioinfo += sizeof(void*);
+	}
+	error("Tweaking file descriptors doesn't work with this MSVCRT.dll");
+	return 1;
+}
+
+static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
+{
+	ioinfo *pioinfo;
+	HANDLE old_handle;
+
+	/* init ioinfo size if we haven't done so */
+	if (init_sizeof_ioinfo())
+		return INVALID_HANDLE_VALUE;
+
+	/* get ioinfo pointer and change the handles */
+	pioinfo = _pioinfo(fd);
+	old_handle = pioinfo->osfhnd;
+	pioinfo->osfhnd = new_handle;
+	return old_handle;
 }
 
 void winansi_init(void)
 {
-	int con1, con2, hwrite_fd;
+	int con1, con2;
 	char name[32];
 
 	/* check if either stdout or stderr is a console output screen buffer */
@@ -516,19 +563,11 @@ void winansi_init(void)
 	if (atexit(winansi_exit))
 		die_errno("atexit(winansi_exit) failed");
 
-	/* create a file descriptor for the write end of the pipe */
-	hwrite_fd = _open_osfhandle((long) duplicate_handle(hwrite), _O_BINARY);
-	if (hwrite_fd == -1)
-		die_errno("_open_osfhandle(%li) failed", (long) hwrite);
-
 	/* redirect stdout / stderr to the pipe */
 	if (con1)
-		hwrite1 = redirect_console(stdout, &hconsole1, hwrite_fd);
+		hconsole1 = swap_osfhnd(1, hwrite1 = duplicate_handle(hwrite));
 	if (con2)
-		hwrite2 = redirect_console(stderr, &hconsole2, hwrite_fd);
-
-	/* close pipe file descriptor (also closes the duped hwrite) */
-	close(hwrite_fd);
+		hconsole2 = swap_osfhnd(2, hwrite2 = duplicate_handle(hwrite));
 }
 
 static int is_same_handle(HANDLE hnd, int fd)
@@ -537,19 +576,6 @@ static int is_same_handle(HANDLE hnd, int fd)
 }
 
 /*
- * Return true if stdout / stderr is a pipe redirecting to the console.
- */
-int winansi_isatty(int fd)
-{
-	if (fd == 1 && is_same_handle(hwrite1, 1))
-		return 1;
-	else if (fd == 2 && is_same_handle(hwrite2, 2))
-		return 1;
-	else
-		return isatty(fd);
-}
-
-/*
  * Returns the real console handle if stdout / stderr is a pipe redirecting
  * to the console. Allows spawn / exec to pass the console to the next process.
  */
-- 
2.0.0.9635.g0be03cb

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: Re: [PATCH 0/5] First part of Unicode console support for msysgit
  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-06 20:48   ` Stepan Kasal
  1 sibling, 0 replies; 22+ messages in thread
From: Stepan Kasal @ 2014-06-06 20:48 UTC (permalink / raw)
  To: Karsten Blees; +Cc: GIT Mailing-list, msysGit

Hello,

On Fri, Jun 06, 2014 at 07:44:33PM +0200, Karsten Blees wrote:
> > Karsten Blees (5):
> >   Support Unicode console output on Windows
> 
> [..] you could have squashed half of
> "Win32: fix segfault in WriteConsoleW when debugging in gdb" [2] (second  half in [5/5]).
> 
> >   Detect console streams more reliably on Windows
> >   Warn if the Windows console font doesn't support Unicode
> 
> I think this one includes
> "MSVC: fix winansi.c compile errors " [3] and
> "Unicode console: fix font warning on Vista and Win7" [4]
[...]
> >   Win32: move main macro to a function
> >   Win32: Thread-safe windows console output

> [2] https://github.com/msysgit/git/commit/cd0792af
> [3] https://github.com/msysgit/git/commit/3abcb04d
> [4] https://github.com/msysgit/git/commit/981aa538

Indeed, you identified them correctly.  And [2] is actually squashed
in [5/5]; I think I can keep it that way.

I'll add the missing fix, take care about original dates, improve the
cover letter (the above links), and resubmit.

Thanks for review this batch of your patches.

Regards,
	Stepan

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2014-06-13 22:09 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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           ` [PATCH 7/6] Win32: reliably detect console pipe handles Karsten Blees
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

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.