{"thread":{"id":"32097","subject":"[PATCH/RFC 0/5] win32: support echo for terminal-prompt","startedAt":"2012-11-13T14:04:02Z","lastAt":"2012-12-04T17:40:44Z","messageCount":27,"participants":["Erik Faye-Lund","Johannes Schindelin","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"203078","messageId":"1352815447-8824-1-git-send-email-kusmabite@gmail.com","threadId":"32097","inReplyTo":null,"subject":"[PATCH/RFC 0/5] win32: support echo for terminal-prompt","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-11-13T14:04:02Z","receivedAt":"2012-11-13T14:04:02Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"We currently only support getpass, which does not echo at all, for\ngit_terminal_prompt on Windows. The Windows console is perfectly\ncapable of doing this, so let's make it so.\n\nThis implementation tries to reuse the /dev/tty-code as much as\npossible.\n\nThe big reason that this becomes a bit hairy is that Ctrl+C needs\nto be handled correctly, so we don't leak the console state to a\nnon-echoing setting when a user aborts.\n\nWindows makes this bit a little bit tricky, in that we need to\nimplement SIGINT for fgetc. However, I suspect that this is a good\nthing to do in the first place.\n\nAn earlier iteration was also breifly discussed here:\nhttp://mid.gmane.org/CABPQNSaUCEDU4+2N63n0k_XwSXOP_iFZG3GEYSPSBPcSVV8wRQ@mail.gmail.com\n\nThe series can also be found here, only with an extra patch that\nmakes the (interactive) testing a bit easier:\n\nhttps://github.com/kusma/git/tree/work/terminal-cleanup\n\nErik Faye-Lund (5):\n  mingw: make fgetc raise SIGINT if apropriate\n  compat/terminal: factor out echo-disabling\n  compat/terminal: separate input and output handles\n  mingw: reuse tty-version of git_terminal_prompt\n  mingw: get rid of getpass implementation\n\n compat/mingw.c    |  91 +++++++++++++++++++++++++++-----------\n compat/mingw.h    |   8 +++-\n compat/terminal.c | 129 ++++++++++++++++++++++++++++++++++++++++--------------\n 3 files changed, 169 insertions(+), 59 deletions(-)\n\n-- \n1.8.0.7.gbeffeda\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"203079","messageId":"1352815447-8824-2-git-send-email-kusmabite@gmail.com","threadId":"32097","inReplyTo":"1352815447-8824-1-git-send-email-kusmabite@gmail.com","subject":"[PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-11-13T14:04:03Z","receivedAt":"2012-11-13T14:04:03Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"Set a control-handler to prevent the process from terminating, and\nsimulate SIGINT so it can be handled by a signal-handler as usual.\n\nSigned-off-by: Erik Faye-Lund <kusmabite@gmail.com>\n---\n compat/mingw.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++--------\n compat/mingw.h |  6 +++++\n 2 files changed, 72 insertions(+), 10 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 78e8f54..33ddfdf 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -319,6 +319,31 @@ ssize_t mingw_write(int fd, const void *buf, size_t count)\n \treturn write(fd, buf, min(count, 31 * 1024 * 1024));\n }\n \n+static BOOL WINAPI ctrl_ignore(DWORD type)\n+{\n+\treturn TRUE;\n+}\n+\n+#undef fgetc\n+int mingw_fgetc(FILE *stream)\n+{\n+\tint ch;\n+\tif (!isatty(_fileno(stream)))\n+\t\treturn fgetc(stream);\n+\n+\tSetConsoleCtrlHandler(ctrl_ignore, TRUE);\n+\twhile (1) {\n+\t\tch = fgetc(stream);\n+\t\tif (ch != EOF || GetLastError() != ERROR_OPERATION_ABORTED)\n+\t\t\tbreak;\n+\n+\t\t/* Ctrl+C was pressed, simulate SIGINT and retry */\n+\t\tmingw_raise(SIGINT);\n+\t}\n+\tSetConsoleCtrlHandler(ctrl_ignore, FALSE);\n+\treturn ch;\n+}\n+\n #undef fopen\n FILE *mingw_fopen (const char *filename, const char *otype)\n {\n@@ -1524,7 +1549,7 @@ static HANDLE timer_event;\n static HANDLE timer_thread;\n static int timer_interval;\n static int one_shot;\n-static sig_handler_t timer_fn = SIG_DFL;\n+static sig_handler_t timer_fn = SIG_DFL, sigint_fn = SIG_DFL;\n \n /* The timer works like this:\n  * The thread, ticktack(), is a trivial routine that most of the time\n@@ -1538,13 +1563,7 @@ static sig_handler_t timer_fn = SIG_DFL;\n static unsigned __stdcall ticktack(void *dummy)\n {\n \twhile (WaitForSingleObject(timer_event, timer_interval) == WAIT_TIMEOUT) {\n-\t\tif (timer_fn == SIG_DFL) {\n-\t\t\tif (isatty(STDERR_FILENO))\n-\t\t\t\tfputs(\"Alarm clock\\n\", stderr);\n-\t\t\texit(128 + SIGALRM);\n-\t\t}\n-\t\tif (timer_fn != SIG_IGN)\n-\t\t\ttimer_fn(SIGALRM);\n+\t\tmingw_raise(SIGALRM);\n \t\tif (one_shot)\n \t\t\tbreak;\n \t}\n@@ -1635,12 +1654,49 @@ int sigaction(int sig, struct sigaction *in, struct sigaction *out)\n sig_handler_t mingw_signal(int sig, sig_handler_t handler)\n {\n \tsig_handler_t old = timer_fn;\n-\tif (sig != SIGALRM)\n+\n+\tswitch (sig) {\n+\tcase SIGALRM:\n+\t\ttimer_fn = handler;\n+\t\tbreak;\n+\n+\tcase SIGINT:\n+\t\tsigint_fn = handler;\n+\t\tbreak;\n+\n+\tdefault:\n \t\treturn signal(sig, handler);\n-\ttimer_fn = handler;\n+\t}\n+\n \treturn old;\n }\n \n+#undef raise\n+int mingw_raise(int sig)\n+{\n+\tswitch (sig) {\n+\tcase SIGALRM:\n+\t\tif (timer_fn == SIG_DFL) {\n+\t\t\tif (isatty(STDERR_FILENO))\n+\t\t\t\tfputs(\"Alarm clock\\n\", stderr);\n+\t\t\texit(128 + SIGALRM);\n+\t\t} else if (timer_fn != SIG_IGN)\n+\t\t\ttimer_fn(SIGALRM);\n+\t\treturn 0;\n+\n+\tcase SIGINT:\n+\t\tif (sigint_fn == SIG_DFL)\n+\t\t\texit(128 + SIGINT);\n+\t\telse if (sigint_fn != SIG_IGN)\n+\t\t\tsigint_fn(SIGINT);\n+\t\treturn 0;\n+\n+\tdefault:\n+\t\treturn raise(sig);\n+\t}\n+}\n+\n+\n static const char *make_backslash_path(const char *path)\n {\n \tstatic char buf[PATH_MAX + 1];\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 61a6521..6b9e69a 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -179,6 +179,9 @@ int mingw_open (const char *filename, int oflags, ...);\n ssize_t mingw_write(int fd, const void *buf, size_t count);\n #define write mingw_write\n \n+int mingw_fgetc(FILE *stream);\n+#define fgetc mingw_fgetc\n+\n FILE *mingw_fopen (const char *filename, const char *otype);\n #define fopen mingw_fopen\n \n@@ -287,6 +290,9 @@ static inline unsigned int git_ntohl(unsigned int x)\n sig_handler_t mingw_signal(int sig, sig_handler_t handler);\n #define signal mingw_signal\n \n+int mingw_raise(int sig);\n+#define raise mingw_raise\n+\n /*\n  * ANSI emulation wrappers\n  */\n-- \n1.8.0.7.gbeffeda\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"203080","messageId":"1352815447-8824-3-git-send-email-kusmabite@gmail.com","threadId":"32097","inReplyTo":"1352815447-8824-1-git-send-email-kusmabite@gmail.com","subject":"[PATCH/RFC 2/5] compat/terminal: factor out echo-disabling","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-11-13T14:04:04Z","receivedAt":"2012-11-13T14:04:04Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"By moving the echo-disabling code to a separate function, we can\nimplement OS-specific versions of it for non-POSIX platforms.\n\nSigned-off-by: Erik Faye-Lund <kusmabite@gmail.com>\n---\n compat/terminal.c | 43 +++++++++++++++++++++++++------------------\n 1 file changed, 25 insertions(+), 18 deletions(-)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex bbb038d..3217838 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -14,6 +14,7 @@ static void restore_term(void)\n \t\treturn;\n \n \ttcsetattr(term_fd, TCSAFLUSH, &old_term);\n+\tclose(term_fd);\n \tterm_fd = -1;\n }\n \n@@ -24,6 +25,27 @@ static void restore_term_on_signal(int sig)\n \traise(sig);\n }\n \n+static int disable_echo()\n+{\n+\tstruct termios t;\n+\n+\tterm_fd = open(\"/dev/tty\", O_RDWR);\n+\tif (tcgetattr(term_fd, &t) < 0)\n+\t\tgoto error;\n+\n+\told_term = t;\n+\tsigchain_push_common(restore_term_on_signal);\n+\n+\tt.c_lflag &= ~ECHO;\n+\tif (!tcsetattr(term_fd, TCSAFLUSH, &t))\n+\t\treturn 0;\n+\n+error:\n+\tclose(term_fd);\n+\tterm_fd = -1;\n+\treturn -1;\n+}\n+\n char *git_terminal_prompt(const char *prompt, int echo)\n {\n \tstatic struct strbuf buf = STRBUF_INIT;\n@@ -34,24 +56,9 @@ char *git_terminal_prompt(const char *prompt, int echo)\n \tif (!fh)\n \t\treturn NULL;\n \n-\tif (!echo) {\n-\t\tstruct termios t;\n-\n-\t\tif (tcgetattr(fileno(fh), &t) < 0) {\n-\t\t\tfclose(fh);\n-\t\t\treturn NULL;\n-\t\t}\n-\n-\t\told_term = t;\n-\t\tterm_fd = fileno(fh);\n-\t\tsigchain_push_common(restore_term_on_signal);\n-\n-\t\tt.c_lflag &= ~ECHO;\n-\t\tif (tcsetattr(fileno(fh), TCSAFLUSH, &t) < 0) {\n-\t\t\tterm_fd = -1;\n-\t\t\tfclose(fh);\n-\t\t\treturn NULL;\n-\t\t}\n+\tif (!echo && disable_echo()) {\n+\t\tfclose(fh);\n+\t\treturn NULL;\n \t}\n \n \tfputs(prompt, fh);\n-- \n1.8.0.7.gbeffeda\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"203081","messageId":"1352815447-8824-4-git-send-email-kusmabite@gmail.com","threadId":"32097","inReplyTo":"1352815447-8824-1-git-send-email-kusmabite@gmail.com","subject":"[PATCH/RFC 3/5] compat/terminal: separate input and output handles","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-11-13T14:04:05Z","receivedAt":"2012-11-13T14:04:05Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Windows, the terminal cannot be opened in read-write mode, so\nwe need distinct pairs for reading and writing. Since this works\nfine on other platforms as well, always open them in pairs.\n\nSigned-off-by: Erik Faye-Lund <kusmabite@gmail.com>\n---\n compat/terminal.c | 29 ++++++++++++++++++-----------\n 1 file changed, 18 insertions(+), 11 deletions(-)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 3217838..4a1fd3d 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -50,29 +50,36 @@ char *git_terminal_prompt(const char *prompt, int echo)\n {\n \tstatic struct strbuf buf = STRBUF_INIT;\n \tint r;\n-\tFILE *fh;\n+\tFILE *input_fh, *output_fh;\n \n-\tfh = fopen(\"/dev/tty\", \"w+\");\n-\tif (!fh)\n+\tinput_fh = fopen(\"/dev/tty\", \"r\");\n+\tif (!input_fh)\n \t\treturn NULL;\n \n+\toutput_fh = fopen(\"/dev/tty\", \"w\");\n+\tif (!output_fh) {\n+\t\tfclose(input_fh);\n+\t\treturn NULL;\n+\t}\n+\n \tif (!echo && disable_echo()) {\n-\t\tfclose(fh);\n+\t\tfclose(input_fh);\n+\t\tfclose(output_fh);\n \t\treturn NULL;\n \t}\n \n-\tfputs(prompt, fh);\n-\tfflush(fh);\n+\tfputs(prompt, output_fh);\n+\tfflush(output_fh);\n \n-\tr = strbuf_getline(&buf, fh, '\\n');\n+\tr = strbuf_getline(&buf, input_fh, '\\n');\n \tif (!echo) {\n-\t\tfseek(fh, SEEK_CUR, 0);\n-\t\tputc('\\n', fh);\n-\t\tfflush(fh);\n+\t\tputc('\\n', output_fh);\n+\t\tfflush(output_fh);\n \t}\n \n \trestore_term();\n-\tfclose(fh);\n+\tfclose(input_fh);\n+\tfclose(output_fh);\n \n \tif (r == EOF)\n \t\treturn NULL;\n-- \n1.8.0.7.gbeffeda\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"203082","messageId":"1352815447-8824-5-git-send-email-kusmabite@gmail.com","threadId":"32097","inReplyTo":"1352815447-8824-1-git-send-email-kusmabite@gmail.com","subject":"[PATCH/RFC 4/5] mingw: reuse tty-version of git_terminal_prompt","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-11-13T14:04:06Z","receivedAt":"2012-11-13T14:04:06Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"The getpass-implementation we use on Windows isn't at all ideal;\nit works in raw-mode (as opposed to cooked mode), and as a result\ndoes not deal correcly with deletion, arrow-keys etc.\n\nInstead, use cooked mode to read a line at the time, allowing the\nC run-time to process the input properly.\n\nSince we set files to be opened in binary-mode by default on\nWindows, introduce a FORCE_TEXT macro that expands to the \"t\"\nmodifier that forces the terminal to be opened in text-mode so we\ndo not have to deal with CRLF issues.\n\nSigned-off-by: Erik Faye-Lund <kusmabite@gmail.com>\n---\n compat/terminal.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++--------\n 1 file changed, 60 insertions(+), 9 deletions(-)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 4a1fd3d..ce0fbd9 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -3,8 +3,22 @@\n #include \"sigchain.h\"\n #include \"strbuf.h\"\n \n+#if defined(HAVE_DEV_TTY) || defined(WIN32)\n+\n+static void restore_term(void);\n+\n+static void restore_term_on_signal(int sig)\n+{\n+\trestore_term();\n+\tsigchain_pop(sig);\n+\traise(sig);\n+}\n+\n #ifdef HAVE_DEV_TTY\n \n+#define INPUT_PATH \"/dev/tty\"\n+#define OUTPUT_PATH \"/dev/tty\"\n+\n static int term_fd = -1;\n static struct termios old_term;\n \n@@ -18,13 +32,6 @@ static void restore_term(void)\n \tterm_fd = -1;\n }\n \n-static void restore_term_on_signal(int sig)\n-{\n-\trestore_term();\n-\tsigchain_pop(sig);\n-\traise(sig);\n-}\n-\n static int disable_echo()\n {\n \tstruct termios t;\n@@ -46,17 +53,61 @@ error:\n \treturn -1;\n }\n \n+#elif defined(WIN32)\n+\n+#define INPUT_PATH \"CONIN$\"\n+#define OUTPUT_PATH \"CONOUT$\"\n+#define FORCE_TEXT \"t\"\n+\n+static HANDLE hconin = INVALID_HANDLE_VALUE;\n+static DWORD cmode;\n+\n+static void restore_term(void)\n+{\n+\tif (hconin == INVALID_HANDLE_VALUE)\n+\t\treturn;\n+\n+\tSetConsoleMode(hconin, cmode);\n+\tCloseHandle(hconin);\n+\thconin = INVALID_HANDLE_VALUE;\n+}\n+\n+static int disable_echo(void)\n+{\n+\thconin = CreateFile(\"CONIN$\", GENERIC_READ | GENERIC_WRITE,\n+\t    FILE_SHARE_READ, NULL, OPEN_EXISTING,\n+\t    FILE_ATTRIBUTE_NORMAL, NULL);\n+\tif (hconin == INVALID_HANDLE_VALUE)\n+\t\treturn -1;\n+\n+\tGetConsoleMode(hconin, &cmode);\n+\tsigchain_push_common(restore_term_on_signal);\n+\tif (!SetConsoleMode(hconin, cmode & (~ENABLE_ECHO_INPUT))) {\n+\t\tCloseHandle(hconin);\n+\t\thconin = INVALID_HANDLE_VALUE;\n+\t\treturn -1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n+#endif\n+\n+#ifndef FORCE_TEXT\n+#define FORCE_TEXT\n+#endif\n+\n char *git_terminal_prompt(const char *prompt, int echo)\n {\n \tstatic struct strbuf buf = STRBUF_INIT;\n \tint r;\n \tFILE *input_fh, *output_fh;\n \n-\tinput_fh = fopen(\"/dev/tty\", \"r\");\n+\tinput_fh = fopen(INPUT_PATH, \"r\" FORCE_TEXT);\n \tif (!input_fh)\n \t\treturn NULL;\n \n-\toutput_fh = fopen(\"/dev/tty\", \"w\");\n+\toutput_fh = fopen(OUTPUT_PATH, \"w\" FORCE_TEXT);\n \tif (!output_fh) {\n \t\tfclose(input_fh);\n \t\treturn NULL;\n-- \n1.8.0.7.gbeffeda\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"203083","messageId":"1352815447-8824-6-git-send-email-kusmabite@gmail.com","threadId":"32097","inReplyTo":"1352815447-8824-1-git-send-email-kusmabite@gmail.com","subject":"[PATCH/RFC 5/5] mingw: get rid of getpass implementation","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-11-13T14:04:07Z","receivedAt":"2012-11-13T14:04:07Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"There's no remaining call-sites, and as pointed out in the\nprevious commit message, it's not quite ideal. So let's just\nlose it.\n\nSigned-off-by: Erik Faye-Lund <kusmabite@gmail.com>\n---\n compat/mingw.c | 15 ---------------\n compat/mingw.h |  2 --\n 2 files changed, 17 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 33ddfdf..5fc14b7 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1758,21 +1758,6 @@ int link(const char *oldpath, const char *newpath)\n \treturn 0;\n }\n \n-char *getpass(const char *prompt)\n-{\n-\tstruct strbuf buf = STRBUF_INIT;\n-\n-\tfputs(prompt, stderr);\n-\tfor (;;) {\n-\t\tchar c = _getch();\n-\t\tif (c == '\\r' || c == '\\n')\n-\t\t\tbreak;\n-\t\tstrbuf_addch(&buf, c);\n-\t}\n-\tfputs(\"\\n\", stderr);\n-\treturn strbuf_detach(&buf, NULL);\n-}\n-\n pid_t waitpid(pid_t pid, int *status, int options)\n {\n \tHANDLE h = OpenProcess(SYNCHRONIZE | PROCESS_QUERY_INFORMATION,\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 6b9e69a..f494ecb 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -55,8 +55,6 @@ struct passwd {\n \tchar *pw_dir;\n };\n \n-extern char *getpass(const char *prompt);\n-\n typedef void (__cdecl *sig_handler_t)(int);\n struct sigaction {\n \tsig_handler_t sa_handler;\n-- \n1.8.0.7.gbeffeda\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204332","messageId":"CABPQNSZ7YRgvS1=SR=Nx0wcn+NXCVwqjVShk-ug3=yX62RH-oQ@mail.gmail.com","threadId":"32097","inReplyTo":"1352815447-8824-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH/RFC 0/5] win32: support echo for terminal-prompt","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-11-30T10:16:59Z","receivedAt":"2012-11-30T10:16:59Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"Ping?\n\nOn Tue, Nov 13, 2012 at 3:04 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n> We currently only support getpass, which does not echo at all, for\n> git_terminal_prompt on Windows. The Windows console is perfectly\n> capable of doing this, so let's make it so.\n>\n> This implementation tries to reuse the /dev/tty-code as much as\n> possible.\n>\n> The big reason that this becomes a bit hairy is that Ctrl+C needs\n> to be handled correctly, so we don't leak the console state to a\n> non-echoing setting when a user aborts.\n>\n> Windows makes this bit a little bit tricky, in that we need to\n> implement SIGINT for fgetc. However, I suspect that this is a good\n> thing to do in the first place.\n>\n> An earlier iteration was also breifly discussed here:\n> http://mid.gmane.org/CABPQNSaUCEDU4+2N63n0k_XwSXOP_iFZG3GEYSPSBPcSVV8wRQ@mail.gmail.com\n>\n> The series can also be found here, only with an extra patch that\n> makes the (interactive) testing a bit easier:\n>\n> https://github.com/kusma/git/tree/work/terminal-cleanup\n>\n> Erik Faye-Lund (5):\n>   mingw: make fgetc raise SIGINT if apropriate\n>   compat/terminal: factor out echo-disabling\n>   compat/terminal: separate input and output handles\n>   mingw: reuse tty-version of git_terminal_prompt\n>   mingw: get rid of getpass implementation\n>\n>  compat/mingw.c    |  91 +++++++++++++++++++++++++++-----------\n>  compat/mingw.h    |   8 +++-\n>  compat/terminal.c | 129 ++++++++++++++++++++++++++++++++++++++++--------------\n>  3 files changed, 169 insertions(+), 59 deletions(-)\n>\n> --\n> 1.8.0.7.gbeffeda\n>\n"},{"id":"204345","messageId":"alpine.DEB.1.00.1211301857170.31987@s15462909.onlinehome-server.info","threadId":"32097","inReplyTo":"1352815447-8824-2-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2012-11-30T17:58:11Z","receivedAt":"2012-11-30T17:58:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 13 Nov 2012, Erik Faye-Lund wrote:\n\n> Set a control-handler to prevent the process from terminating, and\n> simulate SIGINT so it can be handled by a signal-handler as usual.\n\nOne thing you might want to mention is that the fgetc() handling is not\nthread-safe, and intentionally so: if two threads read from the same\nconsole, we are in trouble anyway.\n\nBTW I like the new mingw_raise() very much!\n\nCiao,\nDscho\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204346","messageId":"alpine.DEB.1.00.1211301858570.31987@s15462909.onlinehome-server.info","threadId":"32097","inReplyTo":"1352815447-8824-3-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH/RFC 2/5] compat/terminal: factor out echo-disabling","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2012-11-30T17:59:30Z","receivedAt":"2012-11-30T17:59:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 13 Nov 2012, Erik Faye-Lund wrote:\n\n> By moving the echo-disabling code to a separate function, we can\n> implement OS-specific versions of it for non-POSIX platforms.\n> \n> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>\n> ---\n>  compat/terminal.c | 43 +++++++++++++++++++++++++------------------\n>  1 file changed, 25 insertions(+), 18 deletions(-)\n> \n> diff --git a/compat/terminal.c b/compat/terminal.c\n> index bbb038d..3217838 100644\n> --- a/compat/terminal.c\n> +++ b/compat/terminal.c\n> @@ -14,6 +14,7 @@ static void restore_term(void)\n>  \t\treturn;\n>  \n>  \ttcsetattr(term_fd, TCSAFLUSH, &old_term);\n> +\tclose(term_fd);\n>  \tterm_fd = -1;\n>  }\n\nThat looks like an independent resource leak fix... correct?\n\nRest looks awsomely fine.\nDscho\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204347","messageId":"alpine.DEB.1.00.1211301901120.31987@s15462909.onlinehome-server.info","threadId":"32097","inReplyTo":"1352815447-8824-5-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH/RFC 4/5] mingw: reuse tty-version of git_terminal_prompt","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2012-11-30T18:05:37Z","receivedAt":"2012-11-30T18:05:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi kusma,\n\nOn Tue, 13 Nov 2012, Erik Faye-Lund wrote:\n\n> The getpass-implementation we use on Windows isn't at all ideal;\n> it works in raw-mode (as opposed to cooked mode), and as a result\n> does not deal correcly with deletion, arrow-keys etc.\n> \n> Instead, use cooked mode to read a line at the time, allowing the\n> C run-time to process the input properly.\n\nAwesome!\n\nThe patch itself has a couple Windows-specific things in compat/terminal.c\nthat I would have loved to see in compat/mingw.c instead, but it looks as\nif we have no choice: restore_term() and disable_echo() need to be\nsubstantially different enough from the implementation in compat/mingw.c.\n\n(Read: I am fine with this patch.)\n\nCiao,\nDscho\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204348","messageId":"alpine.DEB.1.00.1211301906080.31987@s15462909.onlinehome-server.info","threadId":"32097","inReplyTo":"1352815447-8824-6-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH/RFC 5/5] mingw: get rid of getpass implementation","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2012-11-30T18:06:22Z","receivedAt":"2012-11-30T18:06:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi kusma,\n\nOn Tue, 13 Nov 2012, Erik Faye-Lund wrote:\n\n> There's no remaining call-sites, and as pointed out in the\n> previous commit message, it's not quite ideal. So let's just\n> lose it.\n\nAwesome!\nDscho\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204349","messageId":"20121130181119.GA7197@sigill.intra.peff.net","threadId":"32097","inReplyTo":"alpine.DEB.1.00.1211301857170.31987@s15462909.onlinehome-server.info","subject":"Re: [msysGit] [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-30T18:11:20Z","receivedAt":"2012-11-30T18:11:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 30, 2012 at 06:58:11PM +0100, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Tue, 13 Nov 2012, Erik Faye-Lund wrote:\n> \n> > Set a control-handler to prevent the process from terminating, and\n> > simulate SIGINT so it can be handled by a signal-handler as usual.\n> \n> One thing you might want to mention is that the fgetc() handling is not\n> thread-safe, and intentionally so: if two threads read from the same\n> console, we are in trouble anyway.\n\nThat makes sense to me, but I'm confused why it is part of mingw_fgetc,\nwhich could in theory read from arbitrary streams, no? It it is not\nnecessarily a console operation at all. I feel like I'm probably missing\nsomething subtle here...\n\n-Peff\n"},{"id":"204351","messageId":"20121130181932.GB7197@sigill.intra.peff.net","threadId":"32097","inReplyTo":"alpine.DEB.1.00.1211301858570.31987@s15462909.onlinehome-server.info","subject":"Re: [PATCH/RFC 2/5] compat/terminal: factor out echo-disabling","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-30T18:19:32Z","receivedAt":"2012-11-30T18:19:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 30, 2012 at 06:59:30PM +0100, Johannes Schindelin wrote:\n\n> > diff --git a/compat/terminal.c b/compat/terminal.c\n> > index bbb038d..3217838 100644\n> > --- a/compat/terminal.c\n> > +++ b/compat/terminal.c\n> > @@ -14,6 +14,7 @@ static void restore_term(void)\n> >  \t\treturn;\n> >  \n> >  \ttcsetattr(term_fd, TCSAFLUSH, &old_term);\n> > +\tclose(term_fd);\n> >  \tterm_fd = -1;\n> >  }\n> \n> That looks like an independent resource leak fix... correct?\n\nI don't think so. In the current code, term_fd does not take ownership\nof the fd. It is fundamentally part of the FILE* in git_terminal_prompt,\nand is closed when we fclose() that. But in Erik's refactor, we actually\nopen a _second_ descriptor to /dev/tty, which needs to be closed.\n\nI don't think there is any reason that should not work (the terminal\ncharacteristics should be per-tty, not per-descriptor), though it does\nfeel a little hacky to have to open /dev/tty twice.\n\n-Peff\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204352","messageId":"20121130182200.GC7197@sigill.intra.peff.net","threadId":"32097","inReplyTo":"1352815447-8824-4-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH/RFC 3/5] compat/terminal: separate input and output handles","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-30T18:22:00Z","receivedAt":"2012-11-30T18:22:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 13, 2012 at 03:04:05PM +0100, Erik Faye-Lund wrote:\n\n> On Windows, the terminal cannot be opened in read-write mode, so\n> we need distinct pairs for reading and writing. Since this works\n> fine on other platforms as well, always open them in pairs.\n\nLooks OK. We're now opening /dev/tty three separate times in the no-echo\ncase, but it's not like this is in a performance critical loop.\n\n-Peff\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204353","messageId":"20121130182716.GD7197@sigill.intra.peff.net","threadId":"32097","inReplyTo":"1352815447-8824-5-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH/RFC 4/5] mingw: reuse tty-version of git_terminal_prompt","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-30T18:27:16Z","receivedAt":"2012-11-30T18:27:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 13, 2012 at 03:04:06PM +0100, Erik Faye-Lund wrote:\n\n> The getpass-implementation we use on Windows isn't at all ideal;\n> it works in raw-mode (as opposed to cooked mode), and as a result\n> does not deal correcly with deletion, arrow-keys etc.\n> \n> Instead, use cooked mode to read a line at the time, allowing the\n> C run-time to process the input properly.\n> \n> Since we set files to be opened in binary-mode by default on\n> Windows, introduce a FORCE_TEXT macro that expands to the \"t\"\n> modifier that forces the terminal to be opened in text-mode so we\n> do not have to deal with CRLF issues.\n\nI think this is OK. I had originally envisioned just having a separate\n\"#ifdef WIN32\" and not really sharing code at all with /dev/tty because\nI was worried that the conditionals would end up making it hard to read.\nThis is a little more complex than I would have liked, but I do not see\nhow the code sharing could be simplified any more than what you have\ndone, and it is nice to avoid repeating ourselves.\n\n-Peff\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204354","messageId":"20121130182748.GE7197@sigill.intra.peff.net","threadId":"32097","inReplyTo":"1352815447-8824-6-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH/RFC 5/5] mingw: get rid of getpass implementation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-30T18:27:48Z","receivedAt":"2012-11-30T18:27:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 13, 2012 at 03:04:07PM +0100, Erik Faye-Lund wrote:\n\n> There's no remaining call-sites, and as pointed out in the\n> previous commit message, it's not quite ideal. So let's just\n> lose it.\n> \n> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>\n> ---\n>  compat/mingw.c | 15 ---------------\n>  compat/mingw.h |  2 --\n>  2 files changed, 17 deletions(-)\n\nYay!\n\n-Peff\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204355","messageId":"20121130183016.GF7197@sigill.intra.peff.net","threadId":"32097","inReplyTo":"CABPQNSZ7YRgvS1=SR=Nx0wcn+NXCVwqjVShk-ug3=yX62RH-oQ@mail.gmail.com","subject":"Re: [PATCH/RFC 0/5] win32: support echo for terminal-prompt","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-30T18:30:16Z","receivedAt":"2012-11-30T18:30:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 30, 2012 at 11:16:59AM +0100, Erik Faye-Lund wrote:\n\n> Ping?\n\nThanks for the reminder; your initial series came while I was traveling.\n\nI think it looks good. The compat/terminal code ends up a little uglier,\nbut I think you overall did a good job of balancing code reuse across\nplatforms with readability.\n\n-Peff\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204379","messageId":"CABPQNSZaaZsfcC6dHDEhYUDpX4doQC7H=1VHGHuqfU4+m31jkA@mail.gmail.com","threadId":"32097","inReplyTo":"20121130181119.GA7197@sigill.intra.peff.net","subject":"Re: [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-12-01T12:31:23Z","receivedAt":"2012-12-01T12:31:23Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Fri, Nov 30, 2012 at 7:11 PM, Jeff King <peff@peff.net> wrote:\n> On Fri, Nov 30, 2012 at 06:58:11PM +0100, Johannes Schindelin wrote:\n>\n>> Hi,\n>>\n>> On Tue, 13 Nov 2012, Erik Faye-Lund wrote:\n>>\n>> > Set a control-handler to prevent the process from terminating, and\n>> > simulate SIGINT so it can be handled by a signal-handler as usual.\n>>\n>> One thing you might want to mention is that the fgetc() handling is not\n>> thread-safe, and intentionally so: if two threads read from the same\n>> console, we are in trouble anyway.\n>\n> That makes sense to me, but I'm confused why it is part of mingw_fgetc,\n> which could in theory read from arbitrary streams, no? It it is not\n> necessarily a console operation at all. I feel like I'm probably missing\n> something subtle here...\n\nI did add an early out for the non-console cases. Is this what you're\nmissing, perhaps?\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204381","messageId":"CABPQNSaCV820zhJGdW++LMf2U7AeODSbfukix3fMfffmNex4YA@mail.gmail.com","threadId":"32097","inReplyTo":"alpine.DEB.1.00.1211301857170.31987@s15462909.onlinehome-server.info","subject":"Re: [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-12-01T12:36:05Z","receivedAt":"2012-12-01T12:36:05Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Fri, Nov 30, 2012 at 6:58 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n> On Tue, 13 Nov 2012, Erik Faye-Lund wrote:\n>\n>> Set a control-handler to prevent the process from terminating, and\n>> simulate SIGINT so it can be handled by a signal-handler as usual.\n>\n> One thing you might want to mention is that the fgetc() handling is not\n> thread-safe, and intentionally so: if two threads read from the same\n> console, we are in trouble anyway.\n\nI'm not entirely sure if I know what you mean. Do you suggest that two\nthreads can race for setting the console ctrl-handler? I don't think\nthat's the case; \"SetConsoleCtrlHandler(x, TRUE)\" adds a console\nhandler to the handler-chain, and SetConsoleCtrlHandler(x, FALSE)\nremoves it. If two threads add handlers, it is my understanding that\none of them will be run, only to report \"no, no more ctrl-handling\nneeded\". Since both handlers block further ctrl-handling, I don't\nthink there's a problem.\n\nDo you care to clarify what your thread-safety complaint is?\n\n> BTW I like the new mingw_raise() very much!\n\nThanks! I originally implemented it for a different reason, but that\npatch didn't turn out to be useful, so it's nice to finally put it to\nuse ;)\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204382","messageId":"CABPQNSY_vHdvBvU_ezjyOzoZeBJAYTJ2829o6Vxs-nJjQVcvDQ@mail.gmail.com","threadId":"32097","inReplyTo":"alpine.DEB.1.00.1211301858570.31987@s15462909.onlinehome-server.info","subject":"Re: [PATCH/RFC 2/5] compat/terminal: factor out echo-disabling","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-12-01T12:43:12Z","receivedAt":"2012-12-01T12:43:12Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Fri, Nov 30, 2012 at 6:59 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n> On Tue, 13 Nov 2012, Erik Faye-Lund wrote:\n>\n>> By moving the echo-disabling code to a separate function, we can\n>> implement OS-specific versions of it for non-POSIX platforms.\n>>\n>> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>\n>> ---\n>>  compat/terminal.c | 43 +++++++++++++++++++++++++------------------\n>>  1 file changed, 25 insertions(+), 18 deletions(-)\n>>\n>> diff --git a/compat/terminal.c b/compat/terminal.c\n>> index bbb038d..3217838 100644\n>> --- a/compat/terminal.c\n>> +++ b/compat/terminal.c\n>> @@ -14,6 +14,7 @@ static void restore_term(void)\n>>               return;\n>>\n>>       tcsetattr(term_fd, TCSAFLUSH, &old_term);\n>> +     close(term_fd);\n>>       term_fd = -1;\n>>  }\n>\n> That looks like an independent resource leak fix... correct?\n\nIt might look like it, but it's not; term_fd used to be returned by\n\"fileno(fh)\", and fh did get properly closed.\n\nWith my refactoring, disable_echo/restore_term takes opens /dev/tty a\nsecond time, like Jeff points out. And that second file descriptor\nneeds to be closed.\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204395","messageId":"20121201173943.GA18339@sigill.intra.peff.net","threadId":"32097","inReplyTo":"CABPQNSZaaZsfcC6dHDEhYUDpX4doQC7H=1VHGHuqfU4+m31jkA@mail.gmail.com","subject":"Re: [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-01T17:39:43Z","receivedAt":"2012-12-01T17:39:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Dec 01, 2012 at 01:31:23PM +0100, Erik Faye-Lund wrote:\n\n> >> One thing you might want to mention is that the fgetc() handling is not\n> >> thread-safe, and intentionally so: if two threads read from the same\n> >> console, we are in trouble anyway.\n> >\n> > That makes sense to me, but I'm confused why it is part of mingw_fgetc,\n> > which could in theory read from arbitrary streams, no? It it is not\n> > necessarily a console operation at all. I feel like I'm probably missing\n> > something subtle here...\n> \n> I did add an early out for the non-console cases. Is this what you're\n> missing, perhaps?\n\nOops, yes. That is exactly what I was missing. :)\n\nSorry for the noise.\n\n-Peff\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204422","messageId":"7vfw3oka68.fsf@alter.siamese.dyndns.org","threadId":"32097","inReplyTo":"1352815447-8824-2-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-02T10:42:07Z","receivedAt":"2012-12-02T10:42:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund <kusmabite@gmail.com> writes:\n\n> @@ -1538,13 +1563,7 @@ static sig_handler_t timer_fn = SIG_DFL;\n>  static unsigned __stdcall ticktack(void *dummy)\n>  {\n>  \twhile (WaitForSingleObject(timer_event, timer_interval) == WAIT_TIMEOUT) {\n> -\t\tif (timer_fn == SIG_DFL) {\n> -\t\t\tif (isatty(STDERR_FILENO))\n> -\t\t\t\tfputs(\"Alarm clock\\n\", stderr);\n> -\t\t\texit(128 + SIGALRM);\n> -\t\t}\n> -\t\tif (timer_fn != SIG_IGN)\n> -\t\t\ttimer_fn(SIGALRM);\n> +\t\tmingw_raise(SIGALRM);\n>  \t\tif (one_shot)\n>  \t\t\tbreak;\n>  \t}\n\nThis hunk seems to have been based on a slightly newer codebase than\nwhat I have, and I had to wiggle the patch a bit to make the series\napply.  Please double check the result when I push out the 'pu'\nbranch.\n\nThanks.\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204471","messageId":"CABPQNSb2NQesbjKc8LUT0TL4=xrnw2A_sJyjX72j9Je_zX1sQw@mail.gmail.com","threadId":"32097","inReplyTo":"7vfw3oka68.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-12-03T23:29:39Z","receivedAt":"2012-12-03T23:29:39Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Sun, Dec 2, 2012 at 11:42 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Erik Faye-Lund <kusmabite@gmail.com> writes:\n>\n>> @@ -1538,13 +1563,7 @@ static sig_handler_t timer_fn = SIG_DFL;\n>>  static unsigned __stdcall ticktack(void *dummy)\n>>  {\n>>       while (WaitForSingleObject(timer_event, timer_interval) == WAIT_TIMEOUT) {\n>> -             if (timer_fn == SIG_DFL) {\n>> -                     if (isatty(STDERR_FILENO))\n>> -                             fputs(\"Alarm clock\\n\", stderr);\n>> -                     exit(128 + SIGALRM);\n>> -             }\n>> -             if (timer_fn != SIG_IGN)\n>> -                     timer_fn(SIGALRM);\n>> +             mingw_raise(SIGALRM);\n>>               if (one_shot)\n>>                       break;\n>>       }\n>\n> This hunk seems to have been based on a slightly newer codebase than\n> what I have, and I had to wiggle the patch a bit to make the series\n> apply.\n\nHuh, no, it shouldn't be; it's based on 8c7a786 (\"Git 1.8.0\").\n\nOH! I see now what the problem is... I *missed* a patch in the series! :P\n\nThat patch corrected the exit-code for our SIGALRM's SIG_DFL routine;\nthe old code did \"die(\"Alarm\");\", but the new one does \"fputs(\"Alarm\nclock\\n\", stderr); exit(128 + SIGALRM)\"\n\n> Please double check the result when I push out the 'pu'\n> branch.\n\nThe resolution is fine; you effectively got the two commits squashed.\nI'll send out a new version with the extra patch added, and your\nsignature-fixup squashed in, OK?\n"},{"id":"204473","messageId":"7vsj7moebl.fsf@alter.siamese.dyndns.org","threadId":"32097","inReplyTo":"CABPQNSb2NQesbjKc8LUT0TL4=xrnw2A_sJyjX72j9Je_zX1sQw@mail.gmail.com","subject":"Re: [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-04T00:23:26Z","receivedAt":"2012-12-04T00:23:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund <kusmabite@gmail.com> writes:\n\n> That patch corrected the exit-code for our SIGALRM's SIG_DFL routine;\n> the old code did \"die(\"Alarm\");\", but the new one does \"fputs(\"Alarm\n> clock\\n\", stderr); exit(128 + SIGALRM)\"\n>\n>> Please double check the result when I push out the 'pu'\n>> branch.\n>\n> The resolution is fine; you effectively got the two commits squashed.\n> I'll send out a new version with the extra patch added, and your\n> signature-fixup squashed in, OK?\n\nSure; thanks for a prompt response.\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204494","messageId":"alpine.DEB.1.00.1212041809150.31987@s15462909.onlinehome-server.info","threadId":"32097","inReplyTo":"CABPQNSaCV820zhJGdW++LMf2U7AeODSbfukix3fMfffmNex4YA@mail.gmail.com","subject":"Re: [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2012-12-04T17:12:02Z","receivedAt":"2012-12-04T17:12:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi kusma,\n\nOn Sat, 1 Dec 2012, Erik Faye-Lund wrote:\n\n> On Fri, Nov 30, 2012 at 6:58 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> > Hi,\n> >\n> > On Tue, 13 Nov 2012, Erik Faye-Lund wrote:\n> >\n> >> Set a control-handler to prevent the process from terminating, and\n> >> simulate SIGINT so it can be handled by a signal-handler as usual.\n> >\n> > One thing you might want to mention is that the fgetc() handling is not\n> > thread-safe, and intentionally so: if two threads read from the same\n> > console, we are in trouble anyway.\n> \n> I'm not entirely sure if I know what you mean. Do you suggest that two\n> threads can race for setting the console ctrl-handler?\n\nThat was my idea, yes.\n\n> I don't think that's the case; \"SetConsoleCtrlHandler(x, TRUE)\" adds a\n> console handler to the handler-chain, and SetConsoleCtrlHandler(x,\n> FALSE) removes it. If two threads add handlers, it is my understanding\n> that one of them will be run, only to report \"no, no more ctrl-handling\n> needed\". Since both handlers block further ctrl-handling, I don't think\n> there's a problem.\n\nMy idea was that the SetConsoleCtrlHandler(x, FALSE) could remove the\nhandler prematurely iff another thread wanted to install the very same\nhandler (but it was already installed by the first thread).\n\nBut as I said: for this to happen, *two* threads need to want to access\nthe console for reading. In that case we'd be in bigger trouble than\nthread unsafety... We cannot read two passwords from the same console at\nthe same time.\n\nCiao,\nDscho\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204495","messageId":"alpine.DEB.1.00.1212041812160.31987@s15462909.onlinehome-server.info","threadId":"32097","inReplyTo":"CABPQNSY_vHdvBvU_ezjyOzoZeBJAYTJ2829o6Vxs-nJjQVcvDQ@mail.gmail.com","subject":"Re: [PATCH/RFC 2/5] compat/terminal: factor out echo-disabling","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2012-12-04T17:12:28Z","receivedAt":"2012-12-04T17:12:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi kusma,\n\nOn Sat, 1 Dec 2012, Erik Faye-Lund wrote:\n\n> On Fri, Nov 30, 2012 at 6:59 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> > Hi,\n> >\n> > On Tue, 13 Nov 2012, Erik Faye-Lund wrote:\n> >\n> >> By moving the echo-disabling code to a separate function, we can\n> >> implement OS-specific versions of it for non-POSIX platforms.\n> >>\n> >> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>\n> >> ---\n> >>  compat/terminal.c | 43 +++++++++++++++++++++++++------------------\n> >>  1 file changed, 25 insertions(+), 18 deletions(-)\n> >>\n> >> diff --git a/compat/terminal.c b/compat/terminal.c\n> >> index bbb038d..3217838 100644\n> >> --- a/compat/terminal.c\n> >> +++ b/compat/terminal.c\n> >> @@ -14,6 +14,7 @@ static void restore_term(void)\n> >>               return;\n> >>\n> >>       tcsetattr(term_fd, TCSAFLUSH, &old_term);\n> >> +     close(term_fd);\n> >>       term_fd = -1;\n> >>  }\n> >\n> > That looks like an independent resource leak fix... correct?\n> \n> It might look like it, but it's not; term_fd used to be returned by\n> \"fileno(fh)\", and fh did get properly closed.\n> \n> With my refactoring, disable_echo/restore_term takes opens /dev/tty a\n> second time, like Jeff points out. And that second file descriptor\n> needs to be closed.\n\nThanks for clarifying,\nDscho\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204497","messageId":"CABPQNSZFDBCZh-T-HFaK3=LmKik65JoMSr4pWkuNE2VrXoo0_g@mail.gmail.com","threadId":"32097","inReplyTo":"alpine.DEB.1.00.1212041809150.31987@s15462909.onlinehome-server.info","subject":"Re: [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-12-04T17:40:44Z","receivedAt":"2012-12-04T17:40:44Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Dec 4, 2012 at 6:12 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi kusma,\n>\n> On Sat, 1 Dec 2012, Erik Faye-Lund wrote:\n>\n>> On Fri, Nov 30, 2012 at 6:58 PM, Johannes Schindelin\n>> <Johannes.Schindelin@gmx.de> wrote:\n>> > Hi,\n>> >\n>> > On Tue, 13 Nov 2012, Erik Faye-Lund wrote:\n>> >\n>> >> Set a control-handler to prevent the process from terminating, and\n>> >> simulate SIGINT so it can be handled by a signal-handler as usual.\n>> >\n>> > One thing you might want to mention is that the fgetc() handling is not\n>> > thread-safe, and intentionally so: if two threads read from the same\n>> > console, we are in trouble anyway.\n>>\n>> I'm not entirely sure if I know what you mean. Do you suggest that two\n>> threads can race for setting the console ctrl-handler?\n>\n> That was my idea, yes.\n>\n>> I don't think that's the case; \"SetConsoleCtrlHandler(x, TRUE)\" adds a\n>> console handler to the handler-chain, and SetConsoleCtrlHandler(x,\n>> FALSE) removes it. If two threads add handlers, it is my understanding\n>> that one of them will be run, only to report \"no, no more ctrl-handling\n>> needed\". Since both handlers block further ctrl-handling, I don't think\n>> there's a problem.\n>\n> My idea was that the SetConsoleCtrlHandler(x, FALSE) could remove the\n> handler prematurely iff another thread wanted to install the very same\n> handler (but it was already installed by the first thread).\n>\n\nYes, but that's not how SetConsoleCtrlHandler works; if a routine x\ngets added twice, it needs to be removed twice as well to not get\ncalled. This little C-program demonstrates that:\n\n---8<---\n#include <windows.h>\n#include <stdio.h>\n\nBOOL WINAPI HandlerRoutine1(DWORD dwCtrlType)\n{\n        printf(\"Hello from handler1!\\n\");\n        return TRUE;\n}\n\nBOOL WINAPI HandlerRoutine2(DWORD dwCtrlType)\n{\n        printf(\"Hello from handler2!\\n\");\n        return FALSE;\n}\n\nint main()\n{\n        SetConsoleCtrlHandler(HandlerRoutine1, TRUE);\n        SetConsoleCtrlHandler(HandlerRoutine2, TRUE);\n        SetConsoleCtrlHandler(HandlerRoutine2, TRUE);\n        SetConsoleCtrlHandler(HandlerRoutine2, FALSE);\n        GenerateConsoleCtrlEvent(CTRL_C_EVENT, 0);\n        SetConsoleCtrlHandler(HandlerRoutine2, FALSE);\n        SetConsoleCtrlHandler(HandlerRoutine1, FALSE);\n}\n---8<---\n\nThis program outputs:\nHello from handler2!\nHello from handler1!\n\nSo since that other thread would add the console ctrl handler before\nit removed it again, this should still be thread-safe as far as I can\ntell.\n\n> But as I said: for this to happen, *two* threads need to want to access\n> the console for reading. In that case we'd be in bigger trouble than\n> thread unsafety... We cannot read two passwords from the same console at\n> the same time.\n\nI agree with that, but I'm saying that *even if* we didn't have this\nlimitation, the code shouldn't be problematic.\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"}]}