[lxc-devel] [lxc/master] lxc-attach: comment & implement login_tty()

[brauner on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 632 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20160222/9b0de90e/attachment.bin>
-------------- next part --------------
From bb78e23756553d0eb77e5520ae5ef84a1946c3eb Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at mailbox.org>
Date: Mon, 22 Feb 2016 12:27:06 +0100
Subject: [PATCH] lxc-attach: comment & implement login_tty()

- The code required to prepare an fd to act as a login tty is shared among
  pty_on_host_callback() and fork_pty(). This implements login_pty(), a
  minimalistic login_tty() clone, to avoid code redundancy.
- Give pty_in_container() a slightly extended comment.

Signed-off-by: Christian Brauner <christian.brauner at mailbox.org>
---
 src/lxc/lxc_attach.c | 50 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 40 insertions(+), 10 deletions(-)

diff --git a/src/lxc/lxc_attach.c b/src/lxc/lxc_attach.c
index 98e917a..ba36913 100644
--- a/src/lxc/lxc_attach.c
+++ b/src/lxc/lxc_attach.c
@@ -210,6 +210,19 @@ struct wrapargs {
 	int ptyfd;
 };
 
+/* Minimalistic login_tty() implementation. */
+static int login_pty(int fd)
+{
+	setsid();
+	if (ioctl(fd, TIOCSCTTY, NULL) < 0)
+		return -1;
+	if (lxc_console_set_stdfds(fd) < 0)
+		return -1;
+	if (fd > STDERR_FILENO)
+		close(fd);
+	return 0;
+}
+
 /* Minimalistic forkpty() implementation. */
 static pid_t fork_pty(int *masterfd)
 {
@@ -226,11 +239,8 @@ static pid_t fork_pty(int *masterfd)
 		return -1;
 	} else if (pid == 0) {
 		close(master);
-		setsid();
-		if (ioctl(slave, TIOCSCTTY, NULL) < 0)
-			_exit(-1); /* automatically closes fds */
-		if (lxc_console_set_stdfds(slave) < 0)
-			_exit(-1); /* automatically closes fds */
+		if (login_pty(slave) < 0)
+			_exit(-1); /* closes fds */
 		return 0;
 	} else {
 		*masterfd = master;
@@ -239,6 +249,29 @@ static pid_t fork_pty(int *masterfd)
 	}
 }
 
+/*
+ * This is probably redundant but just so that intentions can be checked against
+ * code for future modifications. Here is what this is supposed to achieve:
+ * Since we fork() in pty_in_container() the shell that is run on the slave side
+ * of the pty is the grandchild of c->attach() in main(). But what we probably
+ * are interested in is the exit code of the grandchild. So we wait for the
+ * grandchild to change status and return its exit code from this function. We
+ * thereby make the exit code of the grandchild the exit code of the child and
+ * allow the grandparent to see the exit code of the grandchild by waiting on
+ * the pid of the child to change status:
+ *
+ * grandchild: lxc_attach_run_command()/lxc_attach_run_shell()
+ *
+ * child: pty_in_container()
+ * - perform waitpid() on pid returned by lxc_attach_run_command() or
+ *   lxc_attach_run_shell()
+ *
+ * grandparent: c->attach()
+ * - return pid of pty_in_container() in pid argument.
+ *
+ * main()
+ * - perform waitpid() on pid returned in pid argument of c->attach()
+ */
 static int pty_in_container(void *p)
 {
 	int ret;
@@ -345,16 +378,13 @@ static int pty_on_host_callback(void *p)
 	struct wrapargs *wrap = p;
 
 	close(wrap->console->master);
-	setsid();
-	ioctl(wrap->console->slave, TIOCSCTTY, NULL);
-	if (lxc_console_set_stdfds(wrap->console->slave) < 0)
+	if (login_pty(wrap->console->slave) < 0)
 		return -1;
 
 	if (wrap->command->program)
 		lxc_attach_run_command(wrap->command);
 	else
 		lxc_attach_run_shell(NULL);
-
 	return -1;
 }
 
@@ -465,7 +495,7 @@ int main(int argc, char *argv[])
 		if (access(my_args.lxcpath[0], O_RDWR) < 0) {
 			if (!my_args.quiet)
 				fprintf(stderr, "You lack access to %s\n", my_args.lxcpath[0]);
-			exit(ret);
+			exit(EXIT_FAILURE);
 		}
 	}