RFC patch, 3 partseditor: teach it to protect itself from rogue editors

diff --git a/compat/terminal.c b/compat/terminal.c
index 5b903e7c7e..509f2518d1 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -29,16 +29,31 @@ void restore_term(void)
 		return;
 
 	tcsetattr(term_fd, TCSAFLUSH, &old_term);
+
 	close(term_fd);
 	term_fd = -1;
 }
 
+static int is_controlling_terminal(int fd)
+{
+	return (getpgid(0) == tcgetpgrp(fd));
+}
+
 int save_term(int full_duplex)
 {
 	if (term_fd < 0)
 		term_fd = open("/dev/tty", O_RDWR);
 
-	return (term_fd < 0) ? -1 : tcgetattr(term_fd, &old_term);
+	if (term_fd < 0)
+		return -1;
+
+	if (full_duplex && !is_controlling_terminal(term_fd)) {
+		close(term_fd);
+		term_fd = -1;
+		return -1;
+	}
+
+	return tcgetattr(term_fd, &old_term);
 }
 
 static int disable_bits(tcflag_t bits)

diff --git a/editor.c b/editor.c
index fdd3eeafa9..70d3f80966 100644
--- a/editor.c
+++ b/editor.c
@@ -3,6 +3,7 @@
 #include "strbuf.h"
 #include "run-command.h"
 #include "sigchain.h"
+#include "compat/terminal.h"
 
 #ifndef DEFAULT_EDITOR
 #define DEFAULT_EDITOR "vi"
@@ -47,6 +48,16 @@ const char *git_sequence_editor(void)
 	return editor;
 }
 
+static int prepare_term(const char *editor)
+{
+	int need_saverestore = !strcmp(editor, "vi");
+
+	git_config_get_bool("editor.stty", &need_saverestore);
+	if (need_saverestore)
+		return save_term(1);
+	return 0;
+}
+
 static int launch_specified_editor(const char *editor, const char *path,
 				   struct strbuf *buffer, const char *const *env)
 {
@@ -57,7 +68,7 @@ static int launch_specified_editor(const char *editor, const char *path,
 		struct strbuf realpath = STRBUF_INIT;
 		const char *args[] = { editor, NULL, NULL };
 		struct child_process p = CHILD_PROCESS_INIT;
-		int ret, sig;
+		int ret, sig, need_restore = 0;
 		int print_waiting_for_editor = advice_enabled(ADVICE_WAITING_FOR_EDITOR) && isatty(2);
 
 		if (print_waiting_for_editor) {
@@ -83,7 +94,10 @@ static int launch_specified_editor(const char *editor, const char *path,
 		p.env = env;
 		p.use_shell = 1;
 		p.trace2_child_class = "editor";
+		need_restore = prepare_term(editor);
 		if (start_command(&p) < 0) {
+			if (need_restore)
+				restore_term();
 			strbuf_release(&realpath);
 			return error("unable to start editor '%s'", editor);
 		}
@@ -91,6 +105,8 @@ static int launch_specified_editor(const char *editor, const char *path,
 		sigchain_push(SIGINT, SIG_IGN);
 		sigchain_push(SIGQUIT, SIG_IGN);
 		ret = finish_command(&p);
+		if (need_restore)
+			restore_term();
 		strbuf_release(&realpath);
 		sig = ret - 128;
 		sigchain_pop(SIGINT);

Documentation/config.txt, Documentation/config/editor.txt, editor.c

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1167e88e34..342f3cc183 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -361,6 +361,8 @@ include::config/diff.txt[]
 
 include::config/difftool.txt[]
 
+include::config/editor.txt[]
+
 include::config/extensions.txt[]
 
 include::config/fastimport.txt[]
diff --git a/Documentation/config/editor.txt b/Documentation/config/editor.txt
new file mode 100644
index 0000000000..b65c2810fa
--- /dev/null
+++ b/Documentation/config/editor.txt
@@ -0,0 +1,8 @@
+editor.stty::
+	A boolean variable that tells git to save and restore its
+	terminal settings when the editor is invoked, to avoid
+	being affected by garbled input/output, if it misbehaves.
++
+It defaults to 'true' if your editor is "vi" (which is
+also the default if there is no `core.editor`), or to
+'false' otherwise.
diff --git a/editor.c b/editor.c
index 70d3f80966..12180039d2 100644
--- a/editor.c
+++ b/editor.c
@@ -50,7 +50,7 @@ const char *git_sequence_editor(void)
 
 static int prepare_term(const char *editor)
 {
-	int need_saverestore = !strcmp(editor, "vi");
+	int need_saverestore = !strcmp(editor, DEFAULT_EDITOR);
 
 	git_config_get_bool("editor.stty", &need_saverestore);
 	if (need_saverestore)
@@ -68,8 +68,10 @@ static int launch_specified_editor(const char *editor, const char *path,
 		struct strbuf realpath = STRBUF_INIT;
 		const char *args[] = { editor, NULL, NULL };
 		struct child_process p = CHILD_PROCESS_INIT;
-		int ret, sig, need_restore = 0;
-		int print_waiting_for_editor = advice_enabled(ADVICE_WAITING_FOR_EDITOR) && isatty(2);
+		int ret, sig, need_restore;
+		int is_interactive = isatty(2);
+		int print_waiting_for_editor = advice_enabled(ADVICE_WAITING_FOR_EDITOR) &&
+						is_interactive;
 
 		if (print_waiting_for_editor) {
 			/*
@@ -94,7 +96,7 @@ static int launch_specified_editor(const char *editor, const char *path,
 		p.env = env;
 		p.use_shell = 1;
 		p.trace2_child_class = "editor";
-		need_restore = prepare_term(editor);
+		need_restore = is_interactive ? prepare_term(editor) : 0;
 		if (start_command(&p) < 0) {
 			if (need_restore)
 				restore_term();