All of lore.kernel.org
 help / color / mirror / Atom feed
* [ptest-runner][PATCH] utils: ensure child can be session leader
@ 2019-06-13 22:39 Randy MacLeod
  2019-06-13 23:01 ` Randy MacLeod
  2019-06-14 14:48 ` [ptest-runner][PATCH v2] 3 old patches + " Randy MacLeod
  0 siblings, 2 replies; 14+ messages in thread
From: Randy MacLeod @ 2019-06-13 22:39 UTC (permalink / raw)
  To: yocto, anibal.limon

From: Sakib Sajal <sakib.sajal@windriver.com>

When running the run-execscript bash ptest as a user rather than root, a warning:
  bash: cannot set terminal process group (16036): Inappropriate ioctl for device
  bash: no job control in this shell
contaminates the bash log files causing the test to fail. This happens only
when run under ptest-runner and not when interactively testing!

The changes made to fix this include:
1. Get the process group id (pgid) before forking,
2. Set the pgid in both the parent and child to avoid a race,
3. Find, open and set permission on the child tty, and
4. Allow the child to attach to controlling tty.

Also add '-lutil' to Makefile. This lib is from libc and provides openpty.

Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com>
Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com>
---
 Makefile |   2 +-
 utils.c  | 100 +++++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 91 insertions(+), 11 deletions(-)

diff --git a/Makefile b/Makefile
index 1bde7be..439eb79 100644
--- a/Makefile
+++ b/Makefile
@@ -29,7 +29,7 @@ TEST_DATA=$(shell echo `pwd`/tests/data)
 all: $(SOURCES) $(EXECUTABLE)
 
 $(EXECUTABLE): $(OBJECTS)
-	$(CC) $(LDFLAGS) $(OBJECTS) -o $@
+	$(CC) $(LDFLAGS) $(OBJECTS) -lutil -o $@
 
 tests: $(TEST_SOURCES) $(TEST_EXECUTABLE)
 
diff --git a/utils.c b/utils.c
index ad737c2..d8977c6 100644
--- a/utils.c
+++ b/utils.c
@@ -1,5 +1,6 @@
 /**
  * Copyright (c) 2016 Intel Corporation
+ * Copyright (C) 2019 Wind River Systems, Inc.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -22,23 +23,29 @@
  */
 
 #define _GNU_SOURCE 
+
 #include <stdio.h>
 
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <grp.h>
 #include <libgen.h>
-#include <signal.h>
 #include <poll.h>
-#include <fcntl.h>
+#include <pty.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <string.h>
 #include <time.h>
-#include <dirent.h>
+#include <unistd.h>
+
+
+#include <sys/ioctl.h>
 #include <sys/resource.h>
+#include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>
-#include <sys/stat.h>
-#include <unistd.h>
-#include <string.h>
-#include <stdlib.h>
 
-#include <errno.h>
 
 #include "ptest_list.h"
 #include "utils.h"
@@ -346,6 +353,51 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
 	return status;
 }
 
+/* get_slave_pty() returns an integer file descriptor.
+ * If it returns < 0, an error has occurred.
+ * Otherwise, it has returned the slave 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 = -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));
+	}
+
+	slave = open(pty_name, O_RDWR);
+	return (slave);
+}
+
+
 int
 run_ptests(struct ptest_list *head, const struct ptest_options opts,
 		const char *progname, FILE *fp, FILE *fp_stderr)
@@ -362,6 +414,8 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 	int timeouted;
 	time_t sttime, entime;
 	int duration;
+	int slave;
+	int pgid = -1;
 
 	if (opts.xml_filename) {
 		xh = xml_create(ptest_list_length(head), opts.xml_filename);
@@ -379,7 +433,6 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 			close(pipefd_stdout[1]);
 			break;
 		}
-
 		fprintf(fp, "START: %s\n", progname);
 		PTEST_LIST_ITERATE_START(head, p);
 			char *ptest_dir = strdup(p->run_ptest);
@@ -388,6 +441,13 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				break;
 			}
 			dirname(ptest_dir);
+			if (ioctl(0, TIOCNOTTY) == -1) {
+				fprintf(fp, "ERROR: Unable to detach from controlling tty, %s\n", strerror(errno));
+			}
+
+			if ((pgid = getpgid(0)) == -1) {
+				fprintf(fp, "ERROR: getpgid() failed, %s\n", strerror(errno));
+			}
 
 			child = fork();
 			if (child == -1) {
@@ -395,13 +455,33 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				rc = -1;
 				break;
 			} else if (child == 0) {
-				setsid();
+				close(0);
+				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));
+				};
+
+				if (setsid() ==  -1) {
+					fprintf(fp, "ERROR: setsid() failed, %s\n", strerror(errno));
+				}
+
+				if (ioctl(0, TIOCSCTTY, NULL) == -1) {
+					fprintf(fp, "ERROR: Unable to attach to controlling tty, %s\n", strerror(errno));
+				}
+
 				run_child(p->run_ptest, pipefd_stdout[1], pipefd_stderr[1]);
+
 			} else {
 				int status;
 				int fds[2]; fds[0] = pipefd_stdout[0]; fds[1] = pipefd_stderr[0];
 				FILE *fps[2]; fps[0] = fp; fps[1] = fp_stderr;
 
+				if (setpgid(child, pgid) == -1) {
+					fprintf(fp, "ERROR: setpgid() failed, %s\n", strerror(errno));
+				};
+
 				sttime = time(NULL);
 				fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, sttime));
 				fprintf(fp, "BEGIN: %s\n", ptest_dir);
-- 
2.17.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [ptest-runner][PATCH] utils: ensure child can be session leader
  2019-06-13 22:39 [ptest-runner][PATCH] utils: ensure child can be session leader Randy MacLeod
@ 2019-06-13 23:01 ` Randy MacLeod
  2019-06-14 14:48 ` [ptest-runner][PATCH v2] 3 old patches + " Randy MacLeod
  1 sibling, 0 replies; 14+ messages in thread
From: Randy MacLeod @ 2019-06-13 23:01 UTC (permalink / raw)
  To: yocto, anibal.limon, Sakib Sajal

On 6/13/19 6:39 PM, Randy MacLeod wrote:
> From: Sakib Sajal <sakib.sajal@windriver.com>

Oops.

Sakib started on this and then had to work on something else
so I finished it up. If needed, I'll send a v2 with me as
the author, since "finishing up" was most of the work.
We're both down as SOBs so whatever works.
-- 
# Randy MacLeod
# Wind River Linux


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [ptest-runner][PATCH v2] 3 old patches + utils: ensure child can be session leader
  2019-06-13 22:39 [ptest-runner][PATCH] utils: ensure child can be session leader Randy MacLeod
  2019-06-13 23:01 ` Randy MacLeod
@ 2019-06-14 14:48 ` Randy MacLeod
  2019-06-14 14:48   ` [ptest-runner][PATCH v2 1/4] utils: Ensure stdout/stderr are flushed Randy MacLeod
                     ` (3 more replies)
  1 sibling, 4 replies; 14+ messages in thread
From: Randy MacLeod @ 2019-06-14 14:48 UTC (permalink / raw)
  To: yocto, anibal.limon

My patch needs Richards previous 3 patches so I've added them here.

I've cleaned up the patch a bit since v1; mostly it's indentation and
other cosmetic changes. I have added a bit more error handling and
I've renamed get_slave_pty to setup_slave_pty to more accurately 
reflect what the function does. Also I've cleaned up the code to only
use open_pty() to get the tty name.

Care to release an update now that oe-core 2.8-M1 is in QA?

../Randy




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [ptest-runner][PATCH v2 1/4] utils: Ensure stdout/stderr are flushed
  2019-06-14 14:48 ` [ptest-runner][PATCH v2] 3 old patches + " Randy MacLeod
@ 2019-06-14 14:48   ` Randy MacLeod
  2019-06-14 14:48   ` [ptest-runner][PATCH v2 2/4] use process groups when spawning Randy MacLeod
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Randy MacLeod @ 2019-06-14 14:48 UTC (permalink / raw)
  To: yocto, anibal.limon

From: Richard Purdie <richard.purdie@linuxfoundation.org>

There is no guarantee that the data written with fwrite will be flushed to the
buffer. If stdout and stderr are the same thing, this could lead to interleaved
writes. The common case is stdout output so flush the output pipes when writing to
stderr. Also flush stdout before the function returns.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Upstream-Status: Pending [code being tested]
---
 utils.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/utils.c b/utils.c
index 6e453a1..9fab6f2 100644
--- a/utils.c
+++ b/utils.c
@@ -316,8 +316,11 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
 			}
 
 			if (pfds[1].revents != 0) {
-				while ((n = read(fds[1], buf, WAIT_CHILD_BUF_MAX_SIZE)) > 0)
+				while ((n = read(fds[1], buf, WAIT_CHILD_BUF_MAX_SIZE)) > 0) {
+					fflush(fps[0]);
 					fwrite(buf, n, 1, fps[1]);
+					fflush(fps[1]);
+				}
 			}
 
 			clock_gettime(clock, &sentinel);
@@ -336,7 +339,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
 			break;
 	}
 
-
+	fflush(fps[0]);
 	return status;
 }
 
-- 
2.17.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [ptest-runner][PATCH v2 2/4] use process groups when spawning
  2019-06-14 14:48 ` [ptest-runner][PATCH v2] 3 old patches + " Randy MacLeod
  2019-06-14 14:48   ` [ptest-runner][PATCH v2 1/4] utils: Ensure stdout/stderr are flushed Randy MacLeod
@ 2019-06-14 14:48   ` Randy MacLeod
  2019-06-14 14:48   ` [ptest-runner][PATCH v2 3/4] utils: Ensure pipes are read after exit Randy MacLeod
  2019-06-14 14:48   ` [ptest-runner][PATCH v2 4/4] utils: ensure child can be session leader Randy MacLeod
  3 siblings, 0 replies; 14+ messages in thread
From: Randy MacLeod @ 2019-06-14 14:48 UTC (permalink / raw)
  To: yocto, anibal.limon

From: Richard Purdie <richard.purdie@linuxfoundation.org>

Rather than just killing the process we've swawned, set the process group
for spawned children and then kill the group of processes.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Upstream-Status: Pending [code being tested]
---
 utils.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/utils.c b/utils.c
index 9fab6f2..86dcdad 100644
--- a/utils.c
+++ b/utils.c
@@ -330,7 +330,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
 			clock_gettime(clock, &time);
 			if ((time.tv_sec - sentinel.tv_sec) > timeout) {
 				*timeouted = 1;
-				kill(pid, SIGKILL);
+				kill(-pid, SIGKILL);
 				waitflags = 0;
 			}
 		}
@@ -392,6 +392,7 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				rc = -1;
 				break;
 			} else if (child == 0) {
+				setsid();
 				run_child(p->run_ptest, pipefd_stdout[1], pipefd_stderr[1]);
 			} else {
 				int status;
-- 
2.17.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [ptest-runner][PATCH v2 3/4] utils: Ensure pipes are read after exit
  2019-06-14 14:48 ` [ptest-runner][PATCH v2] 3 old patches + " Randy MacLeod
  2019-06-14 14:48   ` [ptest-runner][PATCH v2 1/4] utils: Ensure stdout/stderr are flushed Randy MacLeod
  2019-06-14 14:48   ` [ptest-runner][PATCH v2 2/4] use process groups when spawning Randy MacLeod
@ 2019-06-14 14:48   ` Randy MacLeod
  2019-06-14 14:48   ` [ptest-runner][PATCH v2 4/4] utils: ensure child can be session leader Randy MacLeod
  3 siblings, 0 replies; 14+ messages in thread
From: Randy MacLeod @ 2019-06-14 14:48 UTC (permalink / raw)
  To: yocto, anibal.limon

From: Richard Purdie <richard.purdie@linuxfoundation.org>

There was a race in the code where the pipes may not be read after the process has exited
and data may be left behind in them. This change to ordering ensures the pipes are read
after the exit code has been read meaning no data can be left behind and the logs should
be complete.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Upstream-Status: Pending [code being tested]
---
 utils.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/utils.c b/utils.c
index 86dcdad..ad737c2 100644
--- a/utils.c
+++ b/utils.c
@@ -285,6 +285,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
 	struct pollfd pfds[2];
 	struct timespec sentinel;
 	clockid_t clock = CLOCK_MONOTONIC;
+	int looping = 1;
 	int r;
 
 	int status;
@@ -302,9 +303,23 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
 
 	*timeouted = 0;
 
-	while (1) {
+	while (looping) {
 		waitflags = WNOHANG;
 
+		if (timeout >= 0) {
+			struct timespec time;
+
+			clock_gettime(clock, &time);
+			if ((time.tv_sec - sentinel.tv_sec) > timeout) {
+				*timeouted = 1;
+				kill(-pid, SIGKILL);
+				waitflags = 0;
+			}
+		}
+
+		if (waitpid(pid, &status, waitflags) == pid)
+			looping = 0;
+
 		r = poll(pfds, 2, WAIT_CHILD_POLL_TIMEOUT_MS);
 		if (r > 0) {
 			char buf[WAIT_CHILD_BUF_MAX_SIZE];
@@ -324,19 +339,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
 			}
 
 			clock_gettime(clock, &sentinel);
-		} else if (timeout >= 0) {
-			struct timespec time;
-
-			clock_gettime(clock, &time);
-			if ((time.tv_sec - sentinel.tv_sec) > timeout) {
-				*timeouted = 1;
-				kill(-pid, SIGKILL);
-				waitflags = 0;
-			}
 		}
-
-		if (waitpid(pid, &status, waitflags) == pid)
-			break;
 	}
 
 	fflush(fps[0]);
-- 
2.17.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [ptest-runner][PATCH v2 4/4] utils: ensure child can be session leader
  2019-06-14 14:48 ` [ptest-runner][PATCH v2] 3 old patches + " Randy MacLeod
                     ` (2 preceding siblings ...)
  2019-06-14 14:48   ` [ptest-runner][PATCH v2 3/4] utils: Ensure pipes are read after exit Randy MacLeod
@ 2019-06-14 14:48   ` Randy MacLeod
  2019-06-19 17:49     ` Randy MacLeod
  3 siblings, 1 reply; 14+ messages in thread
From: Randy MacLeod @ 2019-06-14 14:48 UTC (permalink / raw)
  To: yocto, anibal.limon

When running the run-execscript bash ptest as a user rather than root, a warning:
  bash: cannot set terminal process group (16036): Inappropriate ioctl for device
  bash: no job control in this shell
contaminates the bash log files causing the test to fail. This happens only
when run under ptest-runner and not when interactively testing!

The changes made to fix this include:
1. Get the process group id (pgid) before forking,
2. Set the pgid in both the parent and child to avoid a race,
3. Find, open and set permission on the child tty, and
4. Allow the child to attach to controlling tty.

Also add '-lutil' to Makefile. This lib is from libc and provides openpty.

Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com>
Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com>
---
 Makefile |   2 +-
 utils.c  | 102 +++++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 92 insertions(+), 12 deletions(-)

diff --git a/Makefile b/Makefile
index 1bde7be..439eb79 100644
--- a/Makefile
+++ b/Makefile
@@ -29,7 +29,7 @@ TEST_DATA=$(shell echo `pwd`/tests/data)
 all: $(SOURCES) $(EXECUTABLE)
 
 $(EXECUTABLE): $(OBJECTS)
-	$(CC) $(LDFLAGS) $(OBJECTS) -o $@
+	$(CC) $(LDFLAGS) $(OBJECTS) -lutil -o $@
 
 tests: $(TEST_SOURCES) $(TEST_EXECUTABLE)
 
diff --git a/utils.c b/utils.c
index ad737c2..f11ce39 100644
--- a/utils.c
+++ b/utils.c
@@ -1,5 +1,6 @@
 /**
  * Copyright (c) 2016 Intel Corporation
+ * Copyright (C) 2019 Wind River Systems, Inc.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -22,23 +23,27 @@
  */
 
 #define _GNU_SOURCE 
+
 #include <stdio.h>
 
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <grp.h>
 #include <libgen.h>
-#include <signal.h>
 #include <poll.h>
-#include <fcntl.h>
+#include <pty.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <string.h>
 #include <time.h>
-#include <dirent.h>
+#include <unistd.h>
+
+#include <sys/ioctl.h>
 #include <sys/resource.h>
+#include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>
-#include <sys/stat.h>
-#include <unistd.h>
-#include <string.h>
-#include <stdlib.h>
-
-#include <errno.h>
 
 #include "ptest_list.h"
 #include "utils.h"
@@ -346,6 +351,53 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
 	return status;
 }
 
+/* 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 = -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)
@@ -362,6 +414,8 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 	int timeouted;
 	time_t sttime, entime;
 	int duration;
+	int slave;
+	int pgid = -1;
 
 	if (opts.xml_filename) {
 		xh = xml_create(ptest_list_length(head), opts.xml_filename);
@@ -379,7 +433,6 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 			close(pipefd_stdout[1]);
 			break;
 		}
-
 		fprintf(fp, "START: %s\n", progname);
 		PTEST_LIST_ITERATE_START(head, p);
 			char *ptest_dir = strdup(p->run_ptest);
@@ -388,6 +441,13 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				break;
 			}
 			dirname(ptest_dir);
+			if (ioctl(0, TIOCNOTTY) == -1) {
+				fprintf(fp, "ERROR: Unable to detach from controlling tty, %s\n", strerror(errno));
+			}
+
+			if ((pgid = getpgid(0)) == -1) {
+				fprintf(fp, "ERROR: getpgid() failed, %s\n", strerror(errno));
+			}
 
 			child = fork();
 			if (child == -1) {
@@ -395,13 +455,33 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				rc = -1;
 				break;
 			} else if (child == 0) {
-				setsid();
+				close(0);
+				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));
+				}
+
+				if (setsid() ==  -1) {
+					fprintf(fp, "ERROR: setsid() failed, %s\n", strerror(errno));
+				}
+
+				if (ioctl(0, TIOCSCTTY, NULL) == -1) {
+					fprintf(fp, "ERROR: Unable to attach to controlling tty, %s\n", strerror(errno));
+				}
+
 				run_child(p->run_ptest, pipefd_stdout[1], pipefd_stderr[1]);
+
 			} else {
 				int status;
 				int fds[2]; fds[0] = pipefd_stdout[0]; fds[1] = pipefd_stderr[0];
 				FILE *fps[2]; fps[0] = fp; fps[1] = fp_stderr;
 
+				if (setpgid(child, pgid) == -1) {
+					fprintf(fp, "ERROR: setpgid() failed, %s\n", strerror(errno));
+				}
+
 				sttime = time(NULL);
 				fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, sttime));
 				fprintf(fp, "BEGIN: %s\n", ptest_dir);
-- 
2.17.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [ptest-runner][PATCH v2 4/4] utils: ensure child can be session leader
  2019-06-14 14:48   ` [ptest-runner][PATCH v2 4/4] utils: ensure child can be session leader Randy MacLeod
@ 2019-06-19 17:49     ` Randy MacLeod
  2019-06-19 19:24       ` richard.purdie
  2019-06-26  1:51       ` Anibal Limon
  0 siblings, 2 replies; 14+ messages in thread
From: Randy MacLeod @ 2019-06-19 17:49 UTC (permalink / raw)
  To: yocto, anibal.limon

On 6/14/19 10:48 AM, Randy MacLeod wrote:
> When running the run-execscript bash ptest as a user rather than root, a warning:
>    bash: cannot set terminal process group (16036): Inappropriate ioctl for device
>    bash: no job control in this shell
> contaminates the bash log files causing the test to fail. This happens only
> when run under ptest-runner and not when interactively testing!
> 
> The changes made to fix this include:
> 1. Get the process group id (pgid) before forking,
> 2. Set the pgid in both the parent and child to avoid a race,
> 3. Find, open and set permission on the child tty, and
> 4. Allow the child to attach to controlling tty.
> 
> Also add '-lutil' to Makefile. This lib is from libc and provides openpty.


Hmmm, I was making the code compile cleanly under clang using
   -Weverything
when I noticed:

1. the 'make check' tests. They still work fine.
2. The './ptest-runner -d tests/data -t 1' tests
    which now generate loads of error like:
     ERROR: Unable to detach from controlling tty, Inappropriate ioctl 
for device

so while this change fixed the bash-ptest, the ptest-runner self-test
it did something wrong.... Ah, I'm calling:
    ioctl(0, TIOCNOTTY) == -1)
repeatedly in the parent so that's what's generating the extra logs.
Fixed locally and I'll send a patch but it's not urgent. Phew! :)

Anibal,

If you could reply to explain your plans for Richard's patches
that would help me figure out when to send the clang warning clean-ups
commits and what commit to base my work on.


../Randy

> 
> Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com>
> Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com>
> ---
>   Makefile |   2 +-
>   utils.c  | 102 +++++++++++++++++++++++++++++++++++++++++++++++++------
>   2 files changed, 92 insertions(+), 12 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 1bde7be..439eb79 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -29,7 +29,7 @@ TEST_DATA=$(shell echo `pwd`/tests/data)
>   all: $(SOURCES) $(EXECUTABLE)
>   
>   $(EXECUTABLE): $(OBJECTS)
> -	$(CC) $(LDFLAGS) $(OBJECTS) -o $@
> +	$(CC) $(LDFLAGS) $(OBJECTS) -lutil -o $@
>   
>   tests: $(TEST_SOURCES) $(TEST_EXECUTABLE)
>   
> diff --git a/utils.c b/utils.c
> index ad737c2..f11ce39 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -1,5 +1,6 @@
>   /**
>    * Copyright (c) 2016 Intel Corporation
> + * Copyright (C) 2019 Wind River Systems, Inc.
>    *
>    * This program is free software; you can redistribute it and/or
>    * modify it under the terms of the GNU General Public License
> @@ -22,23 +23,27 @@
>    */
>   
>   #define _GNU_SOURCE
> +
>   #include <stdio.h>
>   
> +#include <dirent.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <grp.h>
>   #include <libgen.h>
> -#include <signal.h>
>   #include <poll.h>
> -#include <fcntl.h>
> +#include <pty.h>
> +#include <signal.h>
> +#include <stdlib.h>
> +#include <string.h>
>   #include <time.h>
> -#include <dirent.h>
> +#include <unistd.h>
> +
> +#include <sys/ioctl.h>
>   #include <sys/resource.h>
> +#include <sys/stat.h>
>   #include <sys/types.h>
>   #include <sys/wait.h>
> -#include <sys/stat.h>
> -#include <unistd.h>
> -#include <string.h>
> -#include <stdlib.h>
> -
> -#include <errno.h>
>   
>   #include "ptest_list.h"
>   #include "utils.h"
> @@ -346,6 +351,53 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
>   	return status;
>   }
>   
> +/* 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 = -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)
> @@ -362,6 +414,8 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
>   	int timeouted;
>   	time_t sttime, entime;
>   	int duration;
> +	int slave;
> +	int pgid = -1;
>   
>   	if (opts.xml_filename) {
>   		xh = xml_create(ptest_list_length(head), opts.xml_filename);
> @@ -379,7 +433,6 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
>   			close(pipefd_stdout[1]);
>   			break;
>   		}
> -
>   		fprintf(fp, "START: %s\n", progname);
>   		PTEST_LIST_ITERATE_START(head, p);
>   			char *ptest_dir = strdup(p->run_ptest);
> @@ -388,6 +441,13 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
>   				break;
>   			}
>   			dirname(ptest_dir);
> +			if (ioctl(0, TIOCNOTTY) == -1) {
> +				fprintf(fp, "ERROR: Unable to detach from controlling tty, %s\n", strerror(errno));
> +			}
> +
> +			if ((pgid = getpgid(0)) == -1) {
> +				fprintf(fp, "ERROR: getpgid() failed, %s\n", strerror(errno));
> +			}
>   
>   			child = fork();
>   			if (child == -1) {
> @@ -395,13 +455,33 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
>   				rc = -1;
>   				break;
>   			} else if (child == 0) {
> -				setsid();
> +				close(0);
> +				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));
> +				}
> +
> +				if (setsid() ==  -1) {
> +					fprintf(fp, "ERROR: setsid() failed, %s\n", strerror(errno));
> +				}
> +
> +				if (ioctl(0, TIOCSCTTY, NULL) == -1) {
> +					fprintf(fp, "ERROR: Unable to attach to controlling tty, %s\n", strerror(errno));
> +				}
> +
>   				run_child(p->run_ptest, pipefd_stdout[1], pipefd_stderr[1]);
> +
>   			} else {
>   				int status;
>   				int fds[2]; fds[0] = pipefd_stdout[0]; fds[1] = pipefd_stderr[0];
>   				FILE *fps[2]; fps[0] = fp; fps[1] = fp_stderr;
>   
> +				if (setpgid(child, pgid) == -1) {
> +					fprintf(fp, "ERROR: setpgid() failed, %s\n", strerror(errno));
> +				}
> +
>   				sttime = time(NULL);
>   				fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, sttime));
>   				fprintf(fp, "BEGIN: %s\n", ptest_dir);
> 


-- 
# Randy MacLeod
# Wind River Linux


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [ptest-runner][PATCH v2 4/4] utils: ensure child can be session leader
  2019-06-19 17:49     ` Randy MacLeod
@ 2019-06-19 19:24       ` richard.purdie
  2019-06-19 19:54         ` Randy MacLeod
  2019-06-26  1:51       ` Anibal Limon
  1 sibling, 1 reply; 14+ messages in thread
From: richard.purdie @ 2019-06-19 19:24 UTC (permalink / raw)
  To: Randy MacLeod, yocto, anibal.limon

On Wed, 2019-06-19 at 13:49 -0400, Randy MacLeod wrote:
> On 6/14/19 10:48 AM, Randy MacLeod wrote:
> > When running the run-execscript bash ptest as a user rather than
> > root, a warning:
> >    bash: cannot set terminal process group (16036): Inappropriate
> > ioctl for device
> >    bash: no job control in this shell
> > contaminates the bash log files causing the test to fail. This
> > happens only
> > when run under ptest-runner and not when interactively testing!
> > 
> > The changes made to fix this include:
> > 1. Get the process group id (pgid) before forking,
> > 2. Set the pgid in both the parent and child to avoid a race,
> > 3. Find, open and set permission on the child tty, and
> > 4. Allow the child to attach to controlling tty.
> > 
> > Also add '-lutil' to Makefile. This lib is from libc and provides
> > openpty.
> 
> Hmmm, I was making the code compile cleanly under clang using
>    -Weverything
> when I noticed:
> 
> 1. the 'make check' tests. They still work fine.
> 2. The './ptest-runner -d tests/data -t 1' tests
>     which now generate loads of error like:
>      ERROR: Unable to detach from controlling tty, Inappropriate
> ioctl 
> for device

Aha.

Does this mean you get to own:
https://bugzilla.yoctoproject.org/show_bug.cgi?id=13409

:)

> 
> so while this change fixed the bash-ptest, the ptest-runner self-test
> it did something wrong.... Ah, I'm calling:
>     ioctl(0, TIOCNOTTY) == -1)
> repeatedly in the parent so that's what's generating the extra logs.
> Fixed locally and I'll send a patch but it's not urgent. Phew! :)
> 
> Anibal,
> 
> If you could reply to explain your plans for Richard's patches
> that would help me figure out when to send the clang warning clean-
> ups commits and what commit to base my work on.

I think he believed some of them unnecessary, they were a bit belt and
brances but I'm not sure that is a bad thing.

Cheers,

Richard



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [ptest-runner][PATCH v2 4/4] utils: ensure child can be session leader
  2019-06-19 19:24       ` richard.purdie
@ 2019-06-19 19:54         ` Randy MacLeod
  0 siblings, 0 replies; 14+ messages in thread
From: Randy MacLeod @ 2019-06-19 19:54 UTC (permalink / raw)
  To: richard.purdie, yocto, anibal.limon

On 6/19/19 3:24 PM, richard.purdie@linuxfoundation.org wrote:
> On Wed, 2019-06-19 at 13:49 -0400, Randy MacLeod wrote:
>> On 6/14/19 10:48 AM, Randy MacLeod wrote:
>>> When running the run-execscript bash ptest as a user rather than
>>> root, a warning:
>>>     bash: cannot set terminal process group (16036): Inappropriate
>>> ioctl for device
>>>     bash: no job control in this shell
>>> contaminates the bash log files causing the test to fail. This
>>> happens only
>>> when run under ptest-runner and not when interactively testing!
>>>
>>> The changes made to fix this include:
>>> 1. Get the process group id (pgid) before forking,
>>> 2. Set the pgid in both the parent and child to avoid a race,
>>> 3. Find, open and set permission on the child tty, and
>>> 4. Allow the child to attach to controlling tty.
>>>
>>> Also add '-lutil' to Makefile. This lib is from libc and provides
>>> openpty.
>>
>> Hmmm, I was making the code compile cleanly under clang using
>>     -Weverything
>> when I noticed:
>>
>> 1. the 'make check' tests. They still work fine.
>> 2. The './ptest-runner -d tests/data -t 1' tests
>>      which now generate loads of error like:
>>       ERROR: Unable to detach from controlling tty, Inappropriate
>> ioctl
>> for device
> 
> Aha.
> 
> Does this mean you get to own:
> https://bugzilla.yoctoproject.org/show_bug.cgi?id=13409
> 
> :)

Yes, that's likely my fault.
I've taken the defect.

../Randy 'belt and suspenders' MacLeod

> 
>>
>> so while this change fixed the bash-ptest, the ptest-runner self-test
>> it did something wrong.... Ah, I'm calling:
>>      ioctl(0, TIOCNOTTY) == -1)
>> repeatedly in the parent so that's what's generating the extra logs.
>> Fixed locally and I'll send a patch but it's not urgent. Phew! :)
>>
>> Anibal,
>>
>> If you could reply to explain your plans for Richard's patches
>> that would help me figure out when to send the clang warning clean-
>> ups commits and what commit to base my work on.
> 
> I think he believed some of them unnecessary, they were a bit belt and
> brances but I'm not sure that is a bad thing.
> 
> Cheers,
> 
> Richard
> 


-- 
# Randy MacLeod
# Wind River Linux


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [ptest-runner][PATCH v2 4/4] utils: ensure child can be session leader
  2019-06-19 17:49     ` Randy MacLeod
  2019-06-19 19:24       ` richard.purdie
@ 2019-06-26  1:51       ` Anibal Limon
  2019-06-26  6:56         ` richard.purdie
  2019-06-26 15:55         ` Randy MacLeod
  1 sibling, 2 replies; 14+ messages in thread
From: Anibal Limon @ 2019-06-26  1:51 UTC (permalink / raw)
  To: Randy MacLeod; +Cc: yocto

[-- Attachment #1: Type: text/plain, Size: 10527 bytes --]

On Wed, 19 Jun 2019 at 12:50, Randy MacLeod <randy.macleod@windriver.com>
wrote:

> On 6/14/19 10:48 AM, Randy MacLeod wrote:
> > When running the run-execscript bash ptest as a user rather than root, a
> warning:
> >    bash: cannot set terminal process group (16036): Inappropriate ioctl
> for device
> >    bash: no job control in this shell
> > contaminates the bash log files causing the test to fail. This happens
> only
> > when run under ptest-runner and not when interactively testing!
> >
> > The changes made to fix this include:
> > 1. Get the process group id (pgid) before forking,
> > 2. Set the pgid in both the parent and child to avoid a race,
> > 3. Find, open and set permission on the child tty, and
> > 4. Allow the child to attach to controlling tty.
> >
> > Also add '-lutil' to Makefile. This lib is from libc and provides
> openpty.
>
>
> Hmmm, I was making the code compile cleanly under clang using
>    -Weverything
> when I noticed:
>
> 1. the 'make check' tests. They still work fine.
> 2. The './ptest-runner -d tests/data -t 1' tests
>     which now generate loads of error like:
>      ERROR: Unable to detach from controlling tty, Inappropriate ioctl
> for device
>
> so while this change fixed the bash-ptest, the ptest-runner self-test
> it did something wrong.... Ah, I'm calling:
>     ioctl(0, TIOCNOTTY) == -1)
> repeatedly in the parent so that's what's generating the extra logs.
> Fixed locally and I'll send a patch but it's not urgent. Phew! :)
>
> Anibal,
>
> If you could reply to explain your plans for Richard's patches
> that would help me figure out when to send the clang warning clean-ups
> commits and what commit to base my work on.
>

Hi,

I plan to take the Richard patches, He added in the recipe to have real
testing and looks like
there aren't problems related to, Richard can you confirm it?,

Regarding the openpty include, I see some linkage problem when running make
check, proposed fix:

...
--- a/Makefile
+++ b/Makefile
@@ -22,19 +22,20 @@ TEST_SOURCES=tests/main.c tests/ptest_list.c
tests/utils.c $(BASE_SOURCES)
 TEST_OBJECTS=$(TEST_SOURCES:.c=.o)
 TEST_EXECUTABLE=ptest-runner-test
 TEST_LDFLAGS=-lm -lrt -lpthread
-TEST_LIBSTATIC=-lcheck -lsubunit
+TEST_LIBSTATIC=-lutil
+TEST_LIBSTATIC_TEST=$(TEST_LIBSTATIC) -lcheck -lsubunit

 TEST_DATA=$(shell echo `pwd`/tests/data)

 all: $(SOURCES) $(EXECUTABLE)

 $(EXECUTABLE): $(OBJECTS)
-       $(CC) $(LDFLAGS) $(OBJECTS) -lutil -o $@
+       $(CC) $(LDFLAGS) $(OBJECTS) -o $@ $(TEST_LIBSTATIC)

 tests: $(TEST_SOURCES) $(TEST_EXECUTABLE)

 $(TEST_EXECUTABLE): $(TEST_OBJECTS)
-       $(CC) $(LDFLAGS) $(TEST_LDFLAGS) $(TEST_OBJECTS) -o $@
$(TEST_LIBSTATIC)
+       $(CC) $(LDFLAGS) $(TEST_LDFLAGS) $(TEST_OBJECTS) -o $@
$(TEST_LIBSTATIC_TEST)

 check: $(TEST_EXECUTABLE)
        ./$(TEST_EXECUTABLE) -d $(TEST_DATA)
...

Best regards,
Anibal



>
>
> ../Randy
>
> >
> > Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com>
> > Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com>
> > ---
> >   Makefile |   2 +-
> >   utils.c  | 102 +++++++++++++++++++++++++++++++++++++++++++++++++------
> >   2 files changed, 92 insertions(+), 12 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 1bde7be..439eb79 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -29,7 +29,7 @@ TEST_DATA=$(shell echo `pwd`/tests/data)
> >   all: $(SOURCES) $(EXECUTABLE)
> >
> >   $(EXECUTABLE): $(OBJECTS)
> > -     $(CC) $(LDFLAGS) $(OBJECTS) -o $@
> > +     $(CC) $(LDFLAGS) $(OBJECTS) -lutil -o $@
> >
> >   tests: $(TEST_SOURCES) $(TEST_EXECUTABLE)
> >
> > diff --git a/utils.c b/utils.c
> > index ad737c2..f11ce39 100644
> > --- a/utils.c
> > +++ b/utils.c
> > @@ -1,5 +1,6 @@
> >   /**
> >    * Copyright (c) 2016 Intel Corporation
> > + * Copyright (C) 2019 Wind River Systems, Inc.
> >    *
> >    * This program is free software; you can redistribute it and/or
> >    * modify it under the terms of the GNU General Public License
> > @@ -22,23 +23,27 @@
> >    */
> >
> >   #define _GNU_SOURCE
> > +
> >   #include <stdio.h>
> >
> > +#include <dirent.h>
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <grp.h>
> >   #include <libgen.h>
> > -#include <signal.h>
> >   #include <poll.h>
> > -#include <fcntl.h>
> > +#include <pty.h>
> > +#include <signal.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> >   #include <time.h>
> > -#include <dirent.h>
> > +#include <unistd.h>
> > +
> > +#include <sys/ioctl.h>
> >   #include <sys/resource.h>
> > +#include <sys/stat.h>
> >   #include <sys/types.h>
> >   #include <sys/wait.h>
> > -#include <sys/stat.h>
> > -#include <unistd.h>
> > -#include <string.h>
> > -#include <stdlib.h>
> > -
> > -#include <errno.h>
> >
> >   #include "ptest_list.h"
> >   #include "utils.h"
> > @@ -346,6 +351,53 @@ wait_child(const char *ptest_dir, const char
> *run_ptest, pid_t pid,
> >       return status;
> >   }
> >
> > +/* 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 = -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)
> > @@ -362,6 +414,8 @@ run_ptests(struct ptest_list *head, const struct
> ptest_options opts,
> >       int timeouted;
> >       time_t sttime, entime;
> >       int duration;
> > +     int slave;
> > +     int pgid = -1;
> >
> >       if (opts.xml_filename) {
> >               xh = xml_create(ptest_list_length(head),
> opts.xml_filename);
> > @@ -379,7 +433,6 @@ run_ptests(struct ptest_list *head, const struct
> ptest_options opts,
> >                       close(pipefd_stdout[1]);
> >                       break;
> >               }
> > -
> >               fprintf(fp, "START: %s\n", progname);
> >               PTEST_LIST_ITERATE_START(head, p);
> >                       char *ptest_dir = strdup(p->run_ptest);
> > @@ -388,6 +441,13 @@ run_ptests(struct ptest_list *head, const struct
> ptest_options opts,
> >                               break;
> >                       }
> >                       dirname(ptest_dir);
> > +                     if (ioctl(0, TIOCNOTTY) == -1) {
> > +                             fprintf(fp, "ERROR: Unable to detach from
> controlling tty, %s\n", strerror(errno));
> > +                     }
> > +
> > +                     if ((pgid = getpgid(0)) == -1) {
> > +                             fprintf(fp, "ERROR: getpgid() failed,
> %s\n", strerror(errno));
> > +                     }
> >
> >                       child = fork();
> >                       if (child == -1) {
> > @@ -395,13 +455,33 @@ run_ptests(struct ptest_list *head, const struct
> ptest_options opts,
> >                               rc = -1;
> >                               break;
> >                       } else if (child == 0) {
> > -                             setsid();
> > +                             close(0);
> > +                             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));
> > +                             }
> > +
> > +                             if (setsid() ==  -1) {
> > +                                     fprintf(fp, "ERROR: setsid()
> failed, %s\n", strerror(errno));
> > +                             }
> > +
> > +                             if (ioctl(0, TIOCSCTTY, NULL) == -1) {
> > +                                     fprintf(fp, "ERROR: Unable to
> attach to controlling tty, %s\n", strerror(errno));
> > +                             }
> > +
> >                               run_child(p->run_ptest, pipefd_stdout[1],
> pipefd_stderr[1]);
> > +
> >                       } else {
> >                               int status;
> >                               int fds[2]; fds[0] = pipefd_stdout[0];
> fds[1] = pipefd_stderr[0];
> >                               FILE *fps[2]; fps[0] = fp; fps[1] =
> fp_stderr;
> >
> > +                             if (setpgid(child, pgid) == -1) {
> > +                                     fprintf(fp, "ERROR: setpgid()
> failed, %s\n", strerror(errno));
> > +                             }
> > +
> >                               sttime = time(NULL);
> >                               fprintf(fp, "%s\n", get_stime(stime,
> GET_STIME_BUF_SIZE, sttime));
> >                               fprintf(fp, "BEGIN: %s\n", ptest_dir);
> >
>
>
> --
> # Randy MacLeod
> # Wind River Linux
>

[-- Attachment #2: Type: text/html, Size: 13743 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [ptest-runner][PATCH v2 4/4] utils: ensure child can be session leader
  2019-06-26  1:51       ` Anibal Limon
@ 2019-06-26  6:56         ` richard.purdie
  2019-06-26 15:55         ` Randy MacLeod
  1 sibling, 0 replies; 14+ messages in thread
From: richard.purdie @ 2019-06-26  6:56 UTC (permalink / raw)
  To: Anibal Limon, Randy MacLeod; +Cc: yocto

On Tue, 2019-06-25 at 20:51 -0500, Anibal Limon wrote:
> I plan to take the Richard patches, He added in the recipe to have
> real testing and looks like
> there aren't problems related to, Richard can you confirm it?,

We've been running the patches for a while in the recipe (since we last
discussed them) and we haven't seen any problems with them.

Thanks for looking at this, its a good one to get sorted out.

Cheers,

Richard



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [ptest-runner][PATCH v2 4/4] utils: ensure child can be session leader
  2019-06-26  1:51       ` Anibal Limon
  2019-06-26  6:56         ` richard.purdie
@ 2019-06-26 15:55         ` Randy MacLeod
  2019-06-27 15:27           ` Anibal Limon
  1 sibling, 1 reply; 14+ messages in thread
From: Randy MacLeod @ 2019-06-26 15:55 UTC (permalink / raw)
  To: Anibal Limon; +Cc: yocto

On 6/25/19 9:51 PM, Anibal Limon wrote:
> 
> 
> On Wed, 19 Jun 2019 at 12:50, Randy MacLeod <randy.macleod@windriver.com 
> <mailto:randy.macleod@windriver.com>> wrote:
> 
>     On 6/14/19 10:48 AM, Randy MacLeod wrote:
>      > When running the run-execscript bash ptest as a user rather than
>     root, a warning:
>      >    bash: cannot set terminal process group (16036): Inappropriate
>     ioctl for device
>      >    bash: no job control in this shell
>      > contaminates the bash log files causing the test to fail. This
>     happens only
>      > when run under ptest-runner and not when interactively testing!
>      >
>      > The changes made to fix this include:
>      > 1. Get the process group id (pgid) before forking,
>      > 2. Set the pgid in both the parent and child to avoid a race,
>      > 3. Find, open and set permission on the child tty, and
>      > 4. Allow the child to attach to controlling tty.
>      >
>      > Also add '-lutil' to Makefile. This lib is from libc and provides
>     openpty.
> 
> 
>     Hmmm, I was making the code compile cleanly under clang using
>         -Weverything
>     when I noticed:
> 
>     1. the 'make check' tests. They still work fine.
>     2. The './ptest-runner -d tests/data -t 1' tests
>          which now generate loads of error like:
>           ERROR: Unable to detach from controlling tty, Inappropriate ioctl
>     for device
> 
>     so while this change fixed the bash-ptest, the ptest-runner self-test
>     it did something wrong.... Ah, I'm calling:
>          ioctl(0, TIOCNOTTY) == -1)
>     repeatedly in the parent so that's what's generating the extra logs.
>     Fixed locally and I'll send a patch but it's not urgent. Phew! :)
> 
>     Anibal,
> 
>     If you could reply to explain your plans for Richard's patches
>     that would help me figure out when to send the clang warning clean-ups
>     commits and what commit to base my work on.
> 
> 
> Hi,
> 
> I plan to take the Richard patches, He added in the recipe to have real 
> testing and looks like
> there aren't problems related to, Richard can you confirm it?,
> 
> Regarding the openpty include, I see some linkage problem when running 
> make check, proposed fix:

Yes, I had noticed that and fixed it as well.

I'll send my latest patch series once you have merged
Richard's changes into master. Hopefully that will be today... :)

I decided to compile with:
   clang -Weverything
to see if there were any problems and there
were quite a few things to fix. Now, for the most part,
neither clang nor gcc complain about the code.

../Randy

> 
> ...
> --- a/Makefile
> +++ b/Makefile
> @@ -22,19 +22,20 @@ TEST_SOURCES=tests/main.c tests/ptest_list.c 
> tests/utils.c $(BASE_SOURCES)
>   TEST_OBJECTS=$(TEST_SOURCES:.c=.o)
>   TEST_EXECUTABLE=ptest-runner-test
>   TEST_LDFLAGS=-lm -lrt -lpthread
> -TEST_LIBSTATIC=-lcheck -lsubunit
> +TEST_LIBSTATIC=-lutil
> +TEST_LIBSTATIC_TEST=$(TEST_LIBSTATIC) -lcheck -lsubunit
> 
>   TEST_DATA=$(shell echo `pwd`/tests/data)
> 
>   all: $(SOURCES) $(EXECUTABLE)
> 
>   $(EXECUTABLE): $(OBJECTS)
> -       $(CC) $(LDFLAGS) $(OBJECTS) -lutil -o $@
> +       $(CC) $(LDFLAGS) $(OBJECTS) -o $@ $(TEST_LIBSTATIC)
> 
>   tests: $(TEST_SOURCES) $(TEST_EXECUTABLE)
> 
>   $(TEST_EXECUTABLE): $(TEST_OBJECTS)
> -       $(CC) $(LDFLAGS) $(TEST_LDFLAGS) $(TEST_OBJECTS) -o $@ 
> $(TEST_LIBSTATIC)
> +       $(CC) $(LDFLAGS) $(TEST_LDFLAGS) $(TEST_OBJECTS) -o $@ 
> $(TEST_LIBSTATIC_TEST)
> 
>   check: $(TEST_EXECUTABLE)
>          ./$(TEST_EXECUTABLE) -d $(TEST_DATA)
> ...
> 
> Best regards,
> Anibal
> 
> 
> 
>     ../Randy
> 
>      >
>      > Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com
>     <mailto:sakib.sajal@windriver.com>>
>      > Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com
>     <mailto:Randy.MacLeod@windriver.com>>
>      > ---
>      >   Makefile |   2 +-
>      >   utils.c  | 102
>     +++++++++++++++++++++++++++++++++++++++++++++++++------
>      >   2 files changed, 92 insertions(+), 12 deletions(-)
>      >
>      > diff --git a/Makefile b/Makefile
>      > index 1bde7be..439eb79 100644
>      > --- a/Makefile
>      > +++ b/Makefile
>      > @@ -29,7 +29,7 @@ TEST_DATA=$(shell echo `pwd`/tests/data)
>      >   all: $(SOURCES) $(EXECUTABLE)
>      >
>      >   $(EXECUTABLE): $(OBJECTS)
>      > -     $(CC) $(LDFLAGS) $(OBJECTS) -o $@
>      > +     $(CC) $(LDFLAGS) $(OBJECTS) -lutil -o $@
>      >
>      >   tests: $(TEST_SOURCES) $(TEST_EXECUTABLE)
>      >
>      > diff --git a/utils.c b/utils.c
>      > index ad737c2..f11ce39 100644
>      > --- a/utils.c
>      > +++ b/utils.c
>      > @@ -1,5 +1,6 @@
>      >   /**
>      >    * Copyright (c) 2016 Intel Corporation
>      > + * Copyright (C) 2019 Wind River Systems, Inc.
>      >    *
>      >    * This program is free software; you can redistribute it and/or
>      >    * modify it under the terms of the GNU General Public License
>      > @@ -22,23 +23,27 @@
>      >    */
>      >
>      >   #define _GNU_SOURCE
>      > +
>      >   #include <stdio.h>
>      >
>      > +#include <dirent.h>
>      > +#include <errno.h>
>      > +#include <fcntl.h>
>      > +#include <grp.h>
>      >   #include <libgen.h>
>      > -#include <signal.h>
>      >   #include <poll.h>
>      > -#include <fcntl.h>
>      > +#include <pty.h>
>      > +#include <signal.h>
>      > +#include <stdlib.h>
>      > +#include <string.h>
>      >   #include <time.h>
>      > -#include <dirent.h>
>      > +#include <unistd.h>
>      > +
>      > +#include <sys/ioctl.h>
>      >   #include <sys/resource.h>
>      > +#include <sys/stat.h>
>      >   #include <sys/types.h>
>      >   #include <sys/wait.h>
>      > -#include <sys/stat.h>
>      > -#include <unistd.h>
>      > -#include <string.h>
>      > -#include <stdlib.h>
>      > -
>      > -#include <errno.h>
>      >
>      >   #include "ptest_list.h"
>      >   #include "utils.h"
>      > @@ -346,6 +351,53 @@ wait_child(const char *ptest_dir, const char
>     *run_ptest, pid_t pid,
>      >       return status;
>      >   }
>      >
>      > +/* 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 = -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)
>      > @@ -362,6 +414,8 @@ run_ptests(struct ptest_list *head, const
>     struct ptest_options opts,
>      >       int timeouted;
>      >       time_t sttime, entime;
>      >       int duration;
>      > +     int slave;
>      > +     int pgid = -1;
>      >
>      >       if (opts.xml_filename) {
>      >               xh = xml_create(ptest_list_length(head),
>     opts.xml_filename);
>      > @@ -379,7 +433,6 @@ run_ptests(struct ptest_list *head, const
>     struct ptest_options opts,
>      >                       close(pipefd_stdout[1]);
>      >                       break;
>      >               }
>      > -
>      >               fprintf(fp, "START: %s\n", progname);
>      >               PTEST_LIST_ITERATE_START(head, p);
>      >                       char *ptest_dir = strdup(p->run_ptest);
>      > @@ -388,6 +441,13 @@ run_ptests(struct ptest_list *head, const
>     struct ptest_options opts,
>      >                               break;
>      >                       }
>      >                       dirname(ptest_dir);
>      > +                     if (ioctl(0, TIOCNOTTY) == -1) {
>      > +                             fprintf(fp, "ERROR: Unable to
>     detach from controlling tty, %s\n", strerror(errno));
>      > +                     }
>      > +
>      > +                     if ((pgid = getpgid(0)) == -1) {
>      > +                             fprintf(fp, "ERROR: getpgid()
>     failed, %s\n", strerror(errno));
>      > +                     }
>      >
>      >                       child = fork();
>      >                       if (child == -1) {
>      > @@ -395,13 +455,33 @@ run_ptests(struct ptest_list *head, const
>     struct ptest_options opts,
>      >                               rc = -1;
>      >                               break;
>      >                       } else if (child == 0) {
>      > -                             setsid();
>      > +                             close(0);
>      > +                             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));
>      > +                             }
>      > +
>      > +                             if (setsid() ==  -1) {
>      > +                                     fprintf(fp, "ERROR:
>     setsid() failed, %s\n", strerror(errno));
>      > +                             }
>      > +
>      > +                             if (ioctl(0, TIOCSCTTY, NULL) == -1) {
>      > +                                     fprintf(fp, "ERROR: Unable
>     to attach to controlling tty, %s\n", strerror(errno));
>      > +                             }
>      > +
>      >                               run_child(p->run_ptest,
>     pipefd_stdout[1], pipefd_stderr[1]);
>      > +
>      >                       } else {
>      >                               int status;
>      >                               int fds[2]; fds[0] =
>     pipefd_stdout[0]; fds[1] = pipefd_stderr[0];
>      >                               FILE *fps[2]; fps[0] = fp; fps[1] =
>     fp_stderr;
>      >
>      > +                             if (setpgid(child, pgid) == -1) {
>      > +                                     fprintf(fp, "ERROR:
>     setpgid() failed, %s\n", strerror(errno));
>      > +                             }
>      > +
>      >                               sttime = time(NULL);
>      >                               fprintf(fp, "%s\n",
>     get_stime(stime, GET_STIME_BUF_SIZE, sttime));
>      >                               fprintf(fp, "BEGIN: %s\n", ptest_dir);
>      >
> 
> 
>     -- 
>     # Randy MacLeod
>     # Wind River Linux
> 


-- 
# Randy MacLeod
# Wind River Linux


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [ptest-runner][PATCH v2 4/4] utils: ensure child can be session leader
  2019-06-26 15:55         ` Randy MacLeod
@ 2019-06-27 15:27           ` Anibal Limon
  0 siblings, 0 replies; 14+ messages in thread
From: Anibal Limon @ 2019-06-27 15:27 UTC (permalink / raw)
  To: Randy MacLeod; +Cc: yocto

[-- Attachment #1: Type: text/plain, Size: 13283 bytes --]

On Wed, 26 Jun 2019 at 10:56, Randy MacLeod <randy.macleod@windriver.com>
wrote:

> On 6/25/19 9:51 PM, Anibal Limon wrote:
> >
> >
> > On Wed, 19 Jun 2019 at 12:50, Randy MacLeod <randy.macleod@windriver.com
> > <mailto:randy.macleod@windriver.com>> wrote:
> >
> >     On 6/14/19 10:48 AM, Randy MacLeod wrote:
> >      > When running the run-execscript bash ptest as a user rather than
> >     root, a warning:
> >      >    bash: cannot set terminal process group (16036): Inappropriate
> >     ioctl for device
> >      >    bash: no job control in this shell
> >      > contaminates the bash log files causing the test to fail. This
> >     happens only
> >      > when run under ptest-runner and not when interactively testing!
> >      >
> >      > The changes made to fix this include:
> >      > 1. Get the process group id (pgid) before forking,
> >      > 2. Set the pgid in both the parent and child to avoid a race,
> >      > 3. Find, open and set permission on the child tty, and
> >      > 4. Allow the child to attach to controlling tty.
> >      >
> >      > Also add '-lutil' to Makefile. This lib is from libc and provides
> >     openpty.
> >
> >
> >     Hmmm, I was making the code compile cleanly under clang using
> >         -Weverything
> >     when I noticed:
> >
> >     1. the 'make check' tests. They still work fine.
> >     2. The './ptest-runner -d tests/data -t 1' tests
> >          which now generate loads of error like:
> >           ERROR: Unable to detach from controlling tty, Inappropriate
> ioctl
> >     for device
> >
> >     so while this change fixed the bash-ptest, the ptest-runner self-test
> >     it did something wrong.... Ah, I'm calling:
> >          ioctl(0, TIOCNOTTY) == -1)
> >     repeatedly in the parent so that's what's generating the extra logs.
> >     Fixed locally and I'll send a patch but it's not urgent. Phew! :)
> >
> >     Anibal,
> >
> >     If you could reply to explain your plans for Richard's patches
> >     that would help me figure out when to send the clang warning
> clean-ups
> >     commits and what commit to base my work on.
> >
> >
> > Hi,
> >
> > I plan to take the Richard patches, He added in the recipe to have real
> > testing and looks like
> > there aren't problems related to, Richard can you confirm it?,
> >
> > Regarding the openpty include, I see some linkage problem when running
> > make check, proposed fix:
>
> Yes, I had noticed that and fixed it as well.
>
> I'll send my latest patch series once you have merged
> Richard's changes into master. Hopefully that will be today... :)
>

I just merged the changes,

Thanks,
Anibal


>
> I decided to compile with:
>    clang -Weverything
> to see if there were any problems and there
> were quite a few things to fix. Now, for the most part,
> neither clang nor gcc complain about the code.
>
> ../Randy
>
> >
> > ...
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -22,19 +22,20 @@ TEST_SOURCES=tests/main.c tests/ptest_list.c
> > tests/utils.c $(BASE_SOURCES)
> >   TEST_OBJECTS=$(TEST_SOURCES:.c=.o)
> >   TEST_EXECUTABLE=ptest-runner-test
> >   TEST_LDFLAGS=-lm -lrt -lpthread
> > -TEST_LIBSTATIC=-lcheck -lsubunit
> > +TEST_LIBSTATIC=-lutil
> > +TEST_LIBSTATIC_TEST=$(TEST_LIBSTATIC) -lcheck -lsubunit
> >
> >   TEST_DATA=$(shell echo `pwd`/tests/data)
> >
> >   all: $(SOURCES) $(EXECUTABLE)
> >
> >   $(EXECUTABLE): $(OBJECTS)
> > -       $(CC) $(LDFLAGS) $(OBJECTS) -lutil -o $@
> > +       $(CC) $(LDFLAGS) $(OBJECTS) -o $@ $(TEST_LIBSTATIC)
> >
> >   tests: $(TEST_SOURCES) $(TEST_EXECUTABLE)
> >
> >   $(TEST_EXECUTABLE): $(TEST_OBJECTS)
> > -       $(CC) $(LDFLAGS) $(TEST_LDFLAGS) $(TEST_OBJECTS) -o $@
> > $(TEST_LIBSTATIC)
> > +       $(CC) $(LDFLAGS) $(TEST_LDFLAGS) $(TEST_OBJECTS) -o $@
> > $(TEST_LIBSTATIC_TEST)
> >
> >   check: $(TEST_EXECUTABLE)
> >          ./$(TEST_EXECUTABLE) -d $(TEST_DATA)
> > ...
> >
> > Best regards,
> > Anibal
> >
> >
> >
> >     ../Randy
> >
> >      >
> >      > Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com
> >     <mailto:sakib.sajal@windriver.com>>
> >      > Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com
> >     <mailto:Randy.MacLeod@windriver.com>>
> >      > ---
> >      >   Makefile |   2 +-
> >      >   utils.c  | 102
> >     +++++++++++++++++++++++++++++++++++++++++++++++++------
> >      >   2 files changed, 92 insertions(+), 12 deletions(-)
> >      >
> >      > diff --git a/Makefile b/Makefile
> >      > index 1bde7be..439eb79 100644
> >      > --- a/Makefile
> >      > +++ b/Makefile
> >      > @@ -29,7 +29,7 @@ TEST_DATA=$(shell echo `pwd`/tests/data)
> >      >   all: $(SOURCES) $(EXECUTABLE)
> >      >
> >      >   $(EXECUTABLE): $(OBJECTS)
> >      > -     $(CC) $(LDFLAGS) $(OBJECTS) -o $@
> >      > +     $(CC) $(LDFLAGS) $(OBJECTS) -lutil -o $@
> >      >
> >      >   tests: $(TEST_SOURCES) $(TEST_EXECUTABLE)
> >      >
> >      > diff --git a/utils.c b/utils.c
> >      > index ad737c2..f11ce39 100644
> >      > --- a/utils.c
> >      > +++ b/utils.c
> >      > @@ -1,5 +1,6 @@
> >      >   /**
> >      >    * Copyright (c) 2016 Intel Corporation
> >      > + * Copyright (C) 2019 Wind River Systems, Inc.
> >      >    *
> >      >    * This program is free software; you can redistribute it and/or
> >      >    * modify it under the terms of the GNU General Public License
> >      > @@ -22,23 +23,27 @@
> >      >    */
> >      >
> >      >   #define _GNU_SOURCE
> >      > +
> >      >   #include <stdio.h>
> >      >
> >      > +#include <dirent.h>
> >      > +#include <errno.h>
> >      > +#include <fcntl.h>
> >      > +#include <grp.h>
> >      >   #include <libgen.h>
> >      > -#include <signal.h>
> >      >   #include <poll.h>
> >      > -#include <fcntl.h>
> >      > +#include <pty.h>
> >      > +#include <signal.h>
> >      > +#include <stdlib.h>
> >      > +#include <string.h>
> >      >   #include <time.h>
> >      > -#include <dirent.h>
> >      > +#include <unistd.h>
> >      > +
> >      > +#include <sys/ioctl.h>
> >      >   #include <sys/resource.h>
> >      > +#include <sys/stat.h>
> >      >   #include <sys/types.h>
> >      >   #include <sys/wait.h>
> >      > -#include <sys/stat.h>
> >      > -#include <unistd.h>
> >      > -#include <string.h>
> >      > -#include <stdlib.h>
> >      > -
> >      > -#include <errno.h>
> >      >
> >      >   #include "ptest_list.h"
> >      >   #include "utils.h"
> >      > @@ -346,6 +351,53 @@ wait_child(const char *ptest_dir, const char
> >     *run_ptest, pid_t pid,
> >      >       return status;
> >      >   }
> >      >
> >      > +/* 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 = -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)
> >      > @@ -362,6 +414,8 @@ run_ptests(struct ptest_list *head, const
> >     struct ptest_options opts,
> >      >       int timeouted;
> >      >       time_t sttime, entime;
> >      >       int duration;
> >      > +     int slave;
> >      > +     int pgid = -1;
> >      >
> >      >       if (opts.xml_filename) {
> >      >               xh = xml_create(ptest_list_length(head),
> >     opts.xml_filename);
> >      > @@ -379,7 +433,6 @@ run_ptests(struct ptest_list *head, const
> >     struct ptest_options opts,
> >      >                       close(pipefd_stdout[1]);
> >      >                       break;
> >      >               }
> >      > -
> >      >               fprintf(fp, "START: %s\n", progname);
> >      >               PTEST_LIST_ITERATE_START(head, p);
> >      >                       char *ptest_dir = strdup(p->run_ptest);
> >      > @@ -388,6 +441,13 @@ run_ptests(struct ptest_list *head, const
> >     struct ptest_options opts,
> >      >                               break;
> >      >                       }
> >      >                       dirname(ptest_dir);
> >      > +                     if (ioctl(0, TIOCNOTTY) == -1) {
> >      > +                             fprintf(fp, "ERROR: Unable to
> >     detach from controlling tty, %s\n", strerror(errno));
> >      > +                     }
> >      > +
> >      > +                     if ((pgid = getpgid(0)) == -1) {
> >      > +                             fprintf(fp, "ERROR: getpgid()
> >     failed, %s\n", strerror(errno));
> >      > +                     }
> >      >
> >      >                       child = fork();
> >      >                       if (child == -1) {
> >      > @@ -395,13 +455,33 @@ run_ptests(struct ptest_list *head, const
> >     struct ptest_options opts,
> >      >                               rc = -1;
> >      >                               break;
> >      >                       } else if (child == 0) {
> >      > -                             setsid();
> >      > +                             close(0);
> >      > +                             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));
> >      > +                             }
> >      > +
> >      > +                             if (setsid() ==  -1) {
> >      > +                                     fprintf(fp, "ERROR:
> >     setsid() failed, %s\n", strerror(errno));
> >      > +                             }
> >      > +
> >      > +                             if (ioctl(0, TIOCSCTTY, NULL) ==
> -1) {
> >      > +                                     fprintf(fp, "ERROR: Unable
> >     to attach to controlling tty, %s\n", strerror(errno));
> >      > +                             }
> >      > +
> >      >                               run_child(p->run_ptest,
> >     pipefd_stdout[1], pipefd_stderr[1]);
> >      > +
> >      >                       } else {
> >      >                               int status;
> >      >                               int fds[2]; fds[0] =
> >     pipefd_stdout[0]; fds[1] = pipefd_stderr[0];
> >      >                               FILE *fps[2]; fps[0] = fp; fps[1] =
> >     fp_stderr;
> >      >
> >      > +                             if (setpgid(child, pgid) == -1) {
> >      > +                                     fprintf(fp, "ERROR:
> >     setpgid() failed, %s\n", strerror(errno));
> >      > +                             }
> >      > +
> >      >                               sttime = time(NULL);
> >      >                               fprintf(fp, "%s\n",
> >     get_stime(stime, GET_STIME_BUF_SIZE, sttime));
> >      >                               fprintf(fp, "BEGIN: %s\n",
> ptest_dir);
> >      >
> >
> >
> >     --
> >     # Randy MacLeod
> >     # Wind River Linux
> >
>
>
> --
> # Randy MacLeod
> # Wind River Linux
>

[-- Attachment #2: Type: text/html, Size: 18648 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-06-27 15:26 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-13 22:39 [ptest-runner][PATCH] utils: ensure child can be session leader Randy MacLeod
2019-06-13 23:01 ` Randy MacLeod
2019-06-14 14:48 ` [ptest-runner][PATCH v2] 3 old patches + " Randy MacLeod
2019-06-14 14:48   ` [ptest-runner][PATCH v2 1/4] utils: Ensure stdout/stderr are flushed Randy MacLeod
2019-06-14 14:48   ` [ptest-runner][PATCH v2 2/4] use process groups when spawning Randy MacLeod
2019-06-14 14:48   ` [ptest-runner][PATCH v2 3/4] utils: Ensure pipes are read after exit Randy MacLeod
2019-06-14 14:48   ` [ptest-runner][PATCH v2 4/4] utils: ensure child can be session leader Randy MacLeod
2019-06-19 17:49     ` Randy MacLeod
2019-06-19 19:24       ` richard.purdie
2019-06-19 19:54         ` Randy MacLeod
2019-06-26  1:51       ` Anibal Limon
2019-06-26  6:56         ` richard.purdie
2019-06-26 15:55         ` Randy MacLeod
2019-06-27 15:27           ` Anibal Limon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.