Skip to contentSkip to conversation

chardev: don't splatter terminal settings on exit if not previously set

PATCH In review Daniel P. Berrangé 2 participants 2 messages Jun 4, 2018

lore
Share & tools
lore

Daniel P. Berrangé

author
#0 lore raw

PATCH 1/1 chardev: don't splatter terminal settings on exit if not previously set

The stdio chardev finalize method calls term_exit() to restore the original terminal settings that were saved in the "oldtty" global. If the qemu_chr_open_stdio() method exited with an error, we might not have any original terminal settings saved in "oldtty" yet.

eg

  $ qemu-system-x86_64 -monitor stdio -daemonize
  qemu-system-x86_64: -monitor stdio: cannot use stdio with -daemonize

will cause QEMU to splatter the terminal settings with an all-zeros "struct termios", with predictably unpleasant results. Fortunately the existing "stdio_in_use" flag is suitable witness for whether "oldtty" contains settings that need restoring.

Signed-off-by Daniel P. Berrangé <[email protected]>
---
 chardev/char-stdio.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
chardev/char-stdio.c +4 -2
index d83e60e787..8039e85f64 100644
@@ -46,8 +46,10 @@ static bool stdio_echo_state;
 
 static void term_exit(void)
 {
-    tcsetattr(0, TCSANOW, &oldtty);
-    fcntl(0, F_SETFL, old_fd0_flags);
+    if (stdio_in_use) {
+        tcsetattr(0, TCSANOW, &oldtty);
+        fcntl(0, F_SETFL, old_fd0_flags);
+    }
 }
 
 static void qemu_chr_set_echo_stdio(Chardev *chr, bool echo)
Signature · 1 line
2.17.0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 10 lines · Daniel P. Berrangé

The stdio chardev finalize method calls term_exit() to restore the original terminal settings that were saved in the "oldtty" global. If the qemu_chr_open_stdio() method exited with an error, we might not have any original terminal settings saved in "oldtty" yet.

eg

  $ qemu-system-x86_64 -monitor stdio -daemonize
  qemu-system-x86_64: -monitor stdio: cannot use stdio with -daemonize
Quoted 6 lines

will cause QEMU to splatter the terminal settings with an all-zeros "struct termios", with predictably unpleasant results. Fortunately the existing "stdio_in_use" flag is suitable witness for whether "oldtty" contains settings that need restoring.

Signed-off-by Daniel P. Berrangé <[email protected]>
Reviewed-by Marc-André Lureau <[email protected]>
Quoted 24 lines
---
 chardev/char-stdio.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
chardev/char-stdio.c +4 -3
index d83e60e787..8039e85f64 100644
@@ -46,8 +46,10 @@ static bool stdio_echo_state;
 
 static void term_exit(void)
 {
-    tcsetattr(0, TCSANOW, &oldtty);
-    fcntl(0, F_SETFL, old_fd0_flags);
+    if (stdio_in_use) {
+        tcsetattr(0, TCSANOW, &oldtty);
+        fcntl(0, F_SETFL, old_fd0_flags);
+    }
 }
 
 static void qemu_chr_set_echo_stdio(Chardev *chr, bool echo)
--

2.17.0