[ptest-runner 1/2] utils.c: run-ptests improve pseudo-terminal handling

[ptest-runner 1/2] utils.c: run-ptests improve pseudo-terminal handling

[Anibal Limon]

Setup master and slave pty before run ptest, on child attach to the
slave pty and set it as controlling tty and stdin.

See rev 59381a643e1fcce9b6920cb59af9402dd5eb0dfb .

Co-authored-by: Changqing Li <changqing.li@windriver.com>
Signed-off-by: Anibal Limon <anibal@limonsoftware.com>
---
 utils.c | 90 ++++++++++++---------------------------------------------
 1 file changed, 19 insertions(+), 71 deletions(-)

diff --git a/utils.c b/utils.c
index 46918f4..3a35313 100644
--- a/utils.c
+++ b/utils.c
@@ -341,53 +341,6 @@ run_child(char *run_ptest, int fd_stdout, int fd_stderr)
 	/* exit(1); not needed? */
 }
 
-/* Returns an integer file descriptor.
- * If it returns < 0, an error has occurred.
- * Otherwise, it has returned the slave pty file descriptor.
- * fp should be writable, likely stdout/err.
- */
-static int
-setup_slave_pty(FILE *fp) {
-	int pty_master = -1;
-	int pty_slave = -1;
-	char pty_name[256];
-	struct group *gptr;
-	gid_t gid;
-	int slave = -1;
-
-	if (openpty(&pty_master, &pty_slave, pty_name, NULL, NULL) < 0) {
-		fprintf(fp, "ERROR: openpty() failed with: %s.\n", strerror(errno));
-		return -1;
-	}
-
-	if ((gptr = getgrnam(pty_name)) != 0) {
-		gid = gptr->gr_gid;
-	} else {
-		/* If the tty group does not exist, don't change the
-		 * group on the slave pty, only the owner
-		 */
-		gid = (gid_t)-1;
-	}
-
-	/* chown/chmod the corresponding pty, if possible.
-	 * This will only work if the process has root permissions.
-	 */
-	if (chown(pty_name, getuid(), gid) != 0) {
-		fprintf(fp, "ERROR; chown() failed with: %s.\n", strerror(errno));
-	}
-
-	/* Makes the slave read/writeable for the user. */
-	if (chmod(pty_name, S_IRUSR|S_IWUSR) != 0) {
-		fprintf(fp, "ERROR: chmod() failed with: %s.\n", strerror(errno));
-	}
-
-	if ((slave = open(pty_name, O_RDWR)) == -1) {
-		fprintf(fp, "ERROR: open() failed with: %s.\n", strerror(errno));
-	}
-	return (slave);
-}
-
-
 int
 run_ptests(struct ptest_list *head, const struct ptest_options opts,
 		const char *progname, FILE *fp, FILE *fp_stderr)
@@ -406,16 +359,12 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 
 	do
 	{
-		if (isatty(0) && ioctl(0, TIOCNOTTY) == -1) {
-			fprintf(fp, "ERROR: Unable to detach from controlling tty, %s\n", strerror(errno));
-		}
-
 
 		fprintf(fp, "START: %s\n", progname);
 		PTEST_LIST_ITERATE_START(head, p)
 			int pipefd_stdout[2] = {-1, -1};
 			int pipefd_stderr[2] = {-1, -1};
-			int pgid = -1;
+			int pty[2] = {-1, -1};
 
 			if (pipe2(pipefd_stdout, 0) == -1) {
 				rc = -1;
@@ -428,6 +377,16 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				break;
 			}
 
+			if (openpty(&pty[0], &pty[1], NULL, NULL, NULL) < 0) {
+				fprintf(fp, "ERROR: openpty() failed with: %s.\n", strerror(errno));
+				close(pipefd_stderr[PIPE_READ]);
+				close(pipefd_stderr[PIPE_WRITE]);
+				close(pipefd_stdout[PIPE_READ]);
+				close(pipefd_stdout[PIPE_WRITE]);
+				rc = -1;
+				break;
+			}
+
 			char *ptest_dir = strdup(p->run_ptest);
 			if (ptest_dir == NULL) {
 				rc = -1;
@@ -435,38 +394,31 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 			}
 			dirname(ptest_dir);
 
-			if ((pgid = getpgid(0)) == -1) {
-				fprintf(fp, "ERROR: getpgid() failed, %s\n", strerror(errno));
-			}
-
 			pid_t child = fork();
 			if (child == -1) {
 				fprintf(fp, "ERROR: Fork %s\n", strerror(errno));
 				rc = -1;
 				break;
 			} else if (child == 0) {
-				int slave;
-
-				close(0);
 				/* Close read ends of the pipe */
 				do_close(&pipefd_stdout[PIPE_READ]);
 				do_close(&pipefd_stderr[PIPE_READ]);
 
-				if ((slave = setup_slave_pty(fp)) < 0) {
-					fprintf(fp, "ERROR: could not setup pty (%d).", slave);
-				}
-				if (setpgid(0,pgid) == -1) {
-					fprintf(fp, "ERROR: setpgid() failed, %s\n", strerror(errno));
-				}
+				/* Close master pty and set slave pty as stdin */
+				do_close(&pty[0]);
 
 				if (setsid() ==  -1) {
 					fprintf(fp, "ERROR: setsid() failed, %s\n", strerror(errno));
 				}
-
-				if (ioctl(0, TIOCSCTTY, NULL) == -1) {
+				if (ioctl(pty[1], TIOCSCTTY, NULL) == -1) {
 					fprintf(fp, "ERROR: Unable to attach to controlling tty, %s\n", strerror(errno));
 				}
 
+				if (dup2(pty[1], STDIN_FILENO) < 0) {
+					fprintf(fp, "ERROR: Unable to dup slave pty to stdin, %s\n", strerror(errno));
+				}
+				do_close(&pty[1]);
+
 				if (chdir(ptest_dir) == -1) {
 					fprintf(fp, "ERROR: Unable to chdir(%s), %s\n", ptest_dir, strerror(errno));
 				} else {
@@ -481,10 +433,6 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				do_close(&pipefd_stdout[PIPE_WRITE]);
 				do_close(&pipefd_stderr[PIPE_WRITE]);
 
-				if (setpgid(child, pgid) == -1) {
-					fprintf(fp, "ERROR: setpgid() failed, %s\n", strerror(errno));
-				}
-
 				time_t start_time= time(NULL);
 				fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, start_time));
 				fprintf(fp, "BEGIN: %s\n", ptest_dir);
-- 
2.43.0

[ptest-runner 2/2] utils.c: run_ptests improve error handling on ptests iteration

[Anibal Limon]

If error on setup a new ptest use goto to clean properly and stop
the loop.

Signed-off-by: Anibal Limon <anibal@limonsoftware.com>
---
 utils.c | 51 ++++++++++++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/utils.c b/utils.c
index 3a35313..6cf7705 100644
--- a/utils.c
+++ b/utils.c
@@ -350,7 +350,6 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 
 	struct ptest_list *p;
 
-
 	if (opts.xml_filename) {
 		xh = xml_create(ptest_list_length(head), opts.xml_filename);
 		if (!xh)
@@ -362,43 +361,37 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 
 		fprintf(fp, "START: %s\n", progname);
 		PTEST_LIST_ITERATE_START(head, p)
+			char ptest_dir[PATH_MAX] = {'\0'};
 			int pipefd_stdout[2] = {-1, -1};
 			int pipefd_stderr[2] = {-1, -1};
 			int pty[2] = {-1, -1};
 
+			strcpy(ptest_dir, p->run_ptest);
+			dirname(ptest_dir);
+
 			if (pipe2(pipefd_stdout, 0) == -1) {
+				fprintf(fp, "ERROR: pipe2() failed with: %s.\n", strerror(errno));
 				rc = -1;
-				break;
+				goto ptest_list_fail1;
 			}
+
 			if (pipe2(pipefd_stderr, 0) == -1) {
-				close(pipefd_stdout[PIPE_READ]);
-				close(pipefd_stdout[PIPE_WRITE]);
+				fprintf(fp, "ERROR: pipe2() failed with: %s.\n", strerror(errno));
 				rc = -1;
-				break;
+				goto ptest_list_fail2;
 			}
 
 			if (openpty(&pty[0], &pty[1], NULL, NULL, NULL) < 0) {
 				fprintf(fp, "ERROR: openpty() failed with: %s.\n", strerror(errno));
-				close(pipefd_stderr[PIPE_READ]);
-				close(pipefd_stderr[PIPE_WRITE]);
-				close(pipefd_stdout[PIPE_READ]);
-				close(pipefd_stdout[PIPE_WRITE]);
 				rc = -1;
-				break;
-			}
-
-			char *ptest_dir = strdup(p->run_ptest);
-			if (ptest_dir == NULL) {
-				rc = -1;
-				break;
+				goto ptest_list_fail3;
 			}
-			dirname(ptest_dir);
 
 			pid_t child = fork();
 			if (child == -1) {
 				fprintf(fp, "ERROR: Fork %s\n", strerror(errno));
 				rc = -1;
-				break;
+				goto ptest_list_fail4;
 			} else if (child == 0) {
 				/* Close read ends of the pipe */
 				do_close(&pipefd_stdout[PIPE_READ]);
@@ -424,7 +417,6 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				} else {
 					run_child(p->run_ptest, pipefd_stdout[PIPE_WRITE], pipefd_stderr[PIPE_WRITE]);
 				}
-
 			} else {
 				bool timedout = false;
 				char stime[GET_STIME_BUF_SIZE];
@@ -551,22 +543,31 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				fprintf(fp, "END: %s\n", ptest_dir);
 				fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, end_time));
 			}
-			free(ptest_dir);
-			do_close(&pipefd_stdout[PIPE_READ]);
-			do_close(&pipefd_stdout[PIPE_WRITE]);
+
+ptest_list_fail4:
+			do_close(&pty[0]);
+			do_close(&pty[1]);
+
+ptest_list_fail3:
 			do_close(&pipefd_stderr[PIPE_READ]);
 			do_close(&pipefd_stderr[PIPE_WRITE]);
+ptest_list_fail2:
+			do_close(&pipefd_stdout[PIPE_READ]);
+			do_close(&pipefd_stdout[PIPE_WRITE]);
 
+ptest_list_fail1:
 			fflush(fp);
 			fflush(fp_stderr);
 
+			if (rc == -1) {
+				fprintf(fp_stderr, "run_ptests fails: %s", strerror(errno));
+				break;
+			}
+
 		PTEST_LIST_ITERATE_END
 		fprintf(fp, "STOP: %s\n", progname);
 	} while (0);
 
-	if (rc == -1)
-		fprintf(fp_stderr, "run_ptests fails: %s", strerror(errno));
-
 	if (opts.xml_filename)
 		xml_finish(xh);
 
-- 
2.43.0