Attachment 87351 Details for Bug 70290 – Fix for stdout redirection (bugs 1-2)

[patch] Fix for stdout redirection (bugs 1-2)

systemd-stdout-fix.patch (text/plain), 6.38 KB, created by Luke Shumaker on 2013-10-09 18:20:41 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Luke Shumaker

Created: 2013-10-09 18:20:41 UTC

Size: 6.38 KB

patch

obsolete

>commit 51c54dfaf33ba42ba6fe1268f17ff926d5d0949a
>Author: Luke Shumaker <LukeShu@sbcglobal.net>
>Date:   Tue Oct 8 11:36:05 2013 -0400
>
>    nspawn: Set the size of the PTY based on the size of stdout, not stdin.
>    
>    This was probably originally coded with the assumption that stdin and
>    stdout were the same device, but this clearly is the wrong thing if stdout
>    has been redirected.
>
>diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
>index fc4a8a3..ae4c80d 100644
>--- a/src/nspawn/nspawn.c
>+++ b/src/nspawn/nspawn.c
>@@ -1319,7 +1319,7 @@ int main(int argc, char *argv[]) {
> 
>         log_info("Spawning namespace container on %s (console is %s).", arg_directory, console);
> 
>-        if (ioctl(STDIN_FILENO, TIOCGWINSZ, &ws) >= 0)
>+        if (ioctl(STDOUT_FILENO, TIOCGWINSZ, &ws) >= 0)
>                 ioctl(master, TIOCSWINSZ, &ws);
> 
>         if (unlockpt(master) < 0) {
>
>commit f1efc6b57e7fb6ca8eaf70b8f6cad9c7f01ec48a
>Author: Luke Shumaker <LukeShu@sbcglobal.net>
>Date:   Tue Oct 8 14:22:08 2013 -0400
>
>    nspawn: Don't set the output prop of stdin, nor the input props of stdout.
>    
>    It was calling cfmakeraw(3) on the properties for STDIN_FILENO; cfmakeraw
>    sets both input and output properties.  If (and only if) stdin and stdout
>    are the same device is this correct.  Otherwise, we must change only the
>    input properties of stdin, and only the output properties of stdout.
>    
>    This is mostly straight-forward, but in order to still correctly handle
>    them being the same device, we must apply the changes to the first one (of
>    stdin and stdout) immediately, then detect the current settings for
>    the second, then reset the first, so that changing one doesn't reset
>    the other, when we set them later on.
>    
>      - stdin: detect current settings
>      - stdin: set raw
>      - stdout: detect current settings
>      - stdin: reset to original settings
>      ...
>      - stdin: set raw
>      - stdout: set raw
>      ...
>      - stdin: reset to original settings
>      - stdout: reset to original settings
>
>diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
>index ae4c80d..d1b604b 100644
>--- a/src/nspawn/nspawn.c
>+++ b/src/nspawn/nspawn.c
>@@ -1218,9 +1218,11 @@ int main(int argc, char *argv[]) {
>         _cleanup_close_ int master = -1;
>         int n_fd_passed;
>         const char *console = NULL;
>-        struct termios saved_attr, raw_attr;
>+        struct termios saved_stdin_attr, raw_stdin_attr;
>+        struct termios saved_stdout_attr, raw_stdout_attr;
>         sigset_t mask;
>-        bool saved_attr_valid = false;
>+        bool saved_stdin_attr_valid = false;
>+        bool saved_stdout_attr_valid = false;
>         struct winsize ws;
>         int kmsg_socket_pair[2] = { -1, -1 };
>         FDSet *fds = NULL;
>@@ -1327,13 +1329,27 @@ int main(int argc, char *argv[]) {
>                 goto finish;
>         }
> 
>-        if (tcgetattr(STDIN_FILENO, &saved_attr) >= 0) {
>-                saved_attr_valid = true;
>+        if (tcgetattr(STDIN_FILENO, &saved_stdin_attr) >= 0) {
>+                saved_stdin_attr_valid = true;
> 
>-                raw_attr = saved_attr;
>-                cfmakeraw(&raw_attr);
>-                raw_attr.c_lflag &= ~ECHO;
>+                raw_stdin_attr = saved_stdin_attr;
>+                cfmakeraw(&raw_stdin_attr);
>+                raw_stdin_attr.c_oflag = saved_stdin_attr.c_oflag;
>+                if (tcsetattr(STDIN_FILENO, TCSANOW, &raw_stdin_attr) < 0) {
>+                        log_error("Failed to set stdin terminal attributes: %m");
>+                        goto finish;
>+                }
>+        }
>+        if (tcgetattr(STDOUT_FILENO, &saved_stdout_attr) >= 0) {
>+                saved_stdout_attr_valid = true;
>+
>+                raw_stdout_attr = saved_stdout_attr;
>+                cfmakeraw(&raw_stdout_attr);
>+                raw_stdout_attr.c_iflag = saved_stdout_attr.c_iflag;
>+                raw_stdout_attr.c_lflag = saved_stdout_attr.c_lflag;
>         }
>+        if (saved_stdin_attr_valid)
>+                tcsetattr(STDIN_FILENO, TCSANOW, &saved_stdin_attr);
> 
>         if (socketpair(AF_UNIX, SOCK_DGRAM|SOCK_NONBLOCK|SOCK_CLOEXEC, 0, kmsg_socket_pair) < 0) {
>                 log_error("Failed to create kmsg socket pair.");
>@@ -1402,12 +1418,19 @@ int main(int argc, char *argv[]) {
>                         close_nointr_nofail(master);
>                         master = -1;
> 
>-                        if (saved_attr_valid) {
>-                                if (tcsetattr(STDIN_FILENO, TCSANOW, &raw_attr) < 0) {
>-                                        log_error("Failed to set terminal attributes: %m");
>+                        if (saved_stdout_attr_valid) {
>+                                if (tcsetattr(STDIN_FILENO, TCSANOW, &raw_stdin_attr) < 0) {
>+                                        log_error("Failed to set stdin terminal attributes: %m");
>                                         goto child_fail;
>                                 }
>                         }
>+                        if (saved_stdout_attr_valid) {
>+                                if (tcsetattr(STDOUT_FILENO, TCSANOW, &raw_stdout_attr) < 0) {
>+                                        log_error("Failed to set stdout terminal attributes: %m");
>+                                        goto child_fail;
>+                                }
>+                        }
>+
> 
>                         close_nointr(STDIN_FILENO);
>                         close_nointr(STDOUT_FILENO);
>@@ -1673,8 +1696,10 @@ int main(int argc, char *argv[]) {
>                 if (process_pty(master, pid, &mask) < 0)
>                         goto finish;
> 
>-                if (saved_attr_valid)
>-                        tcsetattr(STDIN_FILENO, TCSANOW, &saved_attr);
>+                if (saved_stdout_attr_valid)
>+                        tcsetattr(STDOUT_FILENO, TCSANOW, &saved_stdout_attr);
>+                if (saved_stdin_attr_valid)
>+                        tcsetattr(STDIN_FILENO, TCSANOW, &saved_stdin_attr);
> 
>                 k = wait_for_terminate(pid, &status);
>                 if (k < 0) {
>@@ -1714,8 +1739,10 @@ int main(int argc, char *argv[]) {
>         }
> 
> finish:
>-        if (saved_attr_valid)
>-                tcsetattr(STDIN_FILENO, TCSANOW, &saved_attr);
>+        if (saved_stdout_attr_valid)
>+                tcsetattr(STDOUT_FILENO, TCSANOW, &saved_stdout_attr);
>+        if (saved_stdin_attr_valid)
>+                tcsetattr(STDIN_FILENO, TCSANOW, &saved_stdin_attr);
> 
>         close_pipe(kmsg_socket_pair);
>

Actions: View | Splinter Review

Attachments on bug 70290: 87300 | 87351

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.