Hi Noah,

Thank you for your review.
I agree with you, CONOUT$ way is much simpler. Please look at the patch.
Regarding msys - yes, that check was not correct.
In fact you can use "con" with msys, if you run sh.exe, not a graphical terminal. So the issue with con not related to msys, but to some terminal implementations. Namely, I see that con is not supported by rxvt, mintty and xterm (from x.cygwin project). (rxvt was default terminal for msys 1.0.10, so I think such behavior was considered as msys feature because of this) Your solution to use IsWindowVisible(GetConsoleWindow()) works for these terminals (I've made simple test and it returns false for all of them), but this check will not work for telnet (console app running through telnet can use con/conout). Maybe this should be considered as a distinct bug with another patch required? (I see no ideal solution for it yet. Probably it's possible to detect not "ostype", but these terminals, though it would not be generic too.) And there is another issue with a console charset. When writing string to a console CRT converts it to console encoding, but when reading input back it doesn't. So it seems, there should be conversion from ConsoleCP() to ACP() and then probably to UTF-8 to make postgres utilities support national chars in passwords or usernames (with createuser --interactive).
I think it can be fixed as another bug too.

Best regards,
Alexander


10.10.2012 15:05, Noah Misch wrote:
Hi Alexander,


The console output code page will usually match the OEM code page, but this is
not guaranteed.  For example, one can change it with chcp.exe before starting
psql.  The conversion should be to the actual console output code page.  After
"chcp 869", notice how printing to stdout yields question marks while your
code yields unrelated characters.

It would be nicer still to find a way to make the output routines treat this
explicitly-opened console like stdout to a console.  I could not find any
documentation around this.  Digging into the CRT source code, I see that the
automatic code page conversion happens in write().  One of tests write() uses
to determine whether the destination is a console is to call GetConsoleMode()
on the HANDLE underlying the CRT file descriptor.  If that call fails, write()
assumes the target is not a console.  GetConsoleMode() requires GENERIC_READ
access on its subject HANDLE, but the HANDLE resulting from our fopen("con",
"w") has only GENERIC_WRITE.  Therefore, write() wrongly concludes that it's
writing to a non-console.  fopen("con", "w+") fails, but fopen("CONOUT$",
"w+") seems to give the outcome we need.  write() recognizes that it's writing
to a console and applies the code page conversion.  Let's use that.

This gave me occasion to look at the special case for MSYS that you mentioned.
I observe the same behavior when running a native psql in a Cygwin xterm;
writes to the console succeed but do not appear anywhere.  Instead of guessing
at console visibility based on an environment variable witnessing a particular
platform, let's check IsWindowVisible(GetConsoleWindow()).

What do you think of taking that approach?

Thanks,
nm

--- a/src/port/sprompt.c
+++ b/src/port/sprompt.c
@@ -60,8 +60,13 @@ simple_prompt(const char *prompt, int maxlen, bool echo)
 	 * Do not try to collapse these into one "w+" mode file. Doesn't work on
 	 * some platforms (eg, HPUX 10.20).
 	 */
+#ifdef WIN32
+	termin = fopen("CONIN$", "r");
+	termout = fopen("CONOUT$", "w+");
+#else
 	termin = fopen(DEVTTY, "r");
 	termout = fopen(DEVTTY, "w");
+#endif
 	if (!termin || !termout
 #ifdef WIN32
 	/* See DEVTTY comment for msys */
-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

Reply via email to