<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 19 Jun 2019 at 12:50, Randy MacLeod <<a href="mailto:randy.macleod@windriver.com" target="_blank">randy.macleod@windriver.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 6/14/19 10:48 AM, Randy MacLeod wrote:<br>
> When running the run-execscript bash ptest as a user rather than root, a warning:<br>
>  Â  bash: cannot set terminal process group (16036): Inappropriate ioctl for device<br>
>  Â  bash: no job control in this shell<br>
> contaminates the bash log files causing the test to fail. This happens only<br>
> when run under ptest-runner and not when interactively testing!<br>
> <br>
> The changes made to fix this include:<br>
> 1. Get the process group id (pgid) before forking,<br>
> 2. Set the pgid in both the parent and child to avoid a race,<br>
> 3. Find, open and set permission on the child tty, and<br>
> 4. Allow the child to attach to controlling tty.<br>
> <br>
> Also add '-lutil' to Makefile. This lib is from libc and provides openpty.<br>
<br>
<br>
Hmmm, I was making the code compile cleanly under clang using<br>
  Â -Weverything<br>
when I noticed:<br>
<br>
1. the 'make check' tests. They still work fine.<br>
2. The './ptest-runner -d tests/data -t 1' tests<br>
  Â  which now generate loads of error like:<br>
  Â  Â ERROR: Unable to detach from controlling tty, Inappropriate ioctl <br>
for device<br>
<br>
so while this change fixed the bash-ptest, the ptest-runner self-test<br>
it did something wrong.... Ah, I'm calling:<br>
  Â  ioctl(0, TIOCNOTTY) == -1)<br>
repeatedly in the parent so that's what's generating the extra logs.<br>
Fixed locally and I'll send a patch but it's not urgent. Phew! :)<br>
<br>
Anibal,<br>
<br>
If you could reply to explain your plans for Richard's patches<br>
that would help me figure out when to send the clang warning clean-ups<br>
commits and what commit to base my work on.<br></blockquote><div><br></div><div>Hi, </div><div><br></div><div>I plan to take the Richard patches, He added in the recipe to have real testing and looks like</div><div>there aren't problems related to, Richard can you confirm it?,</div><div><br></div><div>Regarding the openpty include, I see some linkage problem when running make check, proposed fix:</div><div><br></div><div>...</div><div>--- a/Makefile<br>+++ b/Makefile<br>@@ -22,19 +22,20 @@ TEST_SOURCES=tests/main.c tests/ptest_list.c tests/utils.c $(BASE_SOURCES)<br> TEST_OBJECTS=$(TEST_SOURCES:.c=.o)<br> TEST_EXECUTABLE=ptest-runner-test<br> TEST_LDFLAGS=-lm -lrt -lpthread<br>-TEST_LIBSTATIC=-lcheck -lsubunit<br>+TEST_LIBSTATIC=-lutil<br>+TEST_LIBSTATIC_TEST=$(TEST_LIBSTATIC) -lcheck -lsubunit <br> <br> TEST_DATA=$(shell echo `pwd`/tests/data)<br> <br> all: $(SOURCES) $(EXECUTABLE)<br> <br> $(EXECUTABLE): $(OBJECTS)<br>- Â  Â  Â  $(CC) $(LDFLAGS) $(OBJECTS) -lutil -o $@<br>+ Â  Â  Â  $(CC) $(LDFLAGS) $(OBJECTS) -o $@ $(TEST_LIBSTATIC)<br> <br> tests: $(TEST_SOURCES) $(TEST_EXECUTABLE)<br> <br> $(TEST_EXECUTABLE): $(TEST_OBJECTS)<br>- Â  Â  Â  $(CC) $(LDFLAGS) $(TEST_LDFLAGS) $(TEST_OBJECTS) -o $@ $(TEST_LIBSTATIC)<br>+ Â  Â  Â  $(CC) $(LDFLAGS) $(TEST_LDFLAGS) $(TEST_OBJECTS) -o $@ $(TEST_LIBSTATIC_TEST)<br> <br> check: $(TEST_EXECUTABLE)<br>  Â  Â  Â  ./$(TEST_EXECUTABLE) -d $(TEST_DATA)<br></div><div>...</div><div><br></div><div>Best regards,</div><div>Anibal</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
../Randy<br>
<br>
> <br>
> Signed-off-by: Sakib Sajal <<a href="mailto:sakib.sajal@windriver.com" target="_blank">sakib.sajal@windriver.com</a>><br>
> Signed-off-by: Randy MacLeod <<a href="mailto:Randy.MacLeod@windriver.com" target="_blank">Randy.MacLeod@windriver.com</a>><br>
> ---<br>
>  Â Makefile |  Â 2 +-<br>
>  Â utils.c  | 102 +++++++++++++++++++++++++++++++++++++++++++++++++------<br>
>  Â 2 files changed, 92 insertions(+), 12 deletions(-)<br>
> <br>
> diff --git a/Makefile b/Makefile<br>
> index 1bde7be..439eb79 100644<br>
> --- a/Makefile<br>
> +++ b/Makefile<br>
> @@ -29,7 +29,7 @@ TEST_DATA=$(shell echo `pwd`/tests/data)<br>
>  Â all: $(SOURCES) $(EXECUTABLE)<br>
>  Â <br>
>  Â $(EXECUTABLE): $(OBJECTS)<br>
> -  Â  Â $(CC) $(LDFLAGS) $(OBJECTS) -o $@<br>
> +  Â  Â $(CC) $(LDFLAGS) $(OBJECTS) -lutil -o $@<br>
>  Â <br>
>  Â tests: $(TEST_SOURCES) $(TEST_EXECUTABLE)<br>
>  Â <br>
> diff --git a/utils.c b/utils.c<br>
> index ad737c2..f11ce39 100644<br>
> --- a/utils.c<br>
> +++ b/utils.c<br>
> @@ -1,5 +1,6 @@<br>
>  Â /**<br>
>  Â  * Copyright (c) 2016 Intel Corporation<br>
> + * Copyright (C) 2019 Wind River Systems, Inc.<br>
>  Â  *<br>
>  Â  * This program is free software; you can redistribute it and/or<br>
>  Â  * modify it under the terms of the GNU General Public License<br>
> @@ -22,23 +23,27 @@<br>
>  Â  */<br>
>  Â <br>
>  Â #define _GNU_SOURCE<br>
> +<br>
>  Â #include <stdio.h><br>
>  Â <br>
> +#include <dirent.h><br>
> +#include <errno.h><br>
> +#include <fcntl.h><br>
> +#include <grp.h><br>
>  Â #include <libgen.h><br>
> -#include <signal.h><br>
>  Â #include <poll.h><br>
> -#include <fcntl.h><br>
> +#include <pty.h><br>
> +#include <signal.h><br>
> +#include <stdlib.h><br>
> +#include <string.h><br>
>  Â #include <time.h><br>
> -#include <dirent.h><br>
> +#include <unistd.h><br>
> +<br>
> +#include <sys/ioctl.h><br>
>  Â #include <sys/resource.h><br>
> +#include <sys/stat.h><br>
>  Â #include <sys/types.h><br>
>  Â #include <sys/wait.h><br>
> -#include <sys/stat.h><br>
> -#include <unistd.h><br>
> -#include <string.h><br>
> -#include <stdlib.h><br>
> -<br>
> -#include <errno.h><br>
>  Â <br>
>  Â #include "ptest_list.h"<br>
>  Â #include "utils.h"<br>
> @@ -346,6 +351,53 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,<br>
>  Â  Â  Â return status;<br>
>  Â }<br>
>  Â <br>
> +/* Returns an integer file descriptor.<br>
> + * If it returns < 0, an error has occurred.<br>
> + * Otherwise, it has returned the slave pty file descriptor.<br>
> + * fp should be writable, likely stdout/err.<br>
> + */<br>
> +static int<br>
> +setup_slave_pty(FILE *fp) {<br>
> +  Â  Â int pty_master = -1;<br>
> +  Â  Â int pty_slave = -1;<br>
> +  Â  Â char pty_name[256];<br>
> +  Â  Â struct group *gptr;<br>
> +  Â  Â gid_t gid;<br>
> +  Â  Â int slave = -1;<br>
> +<br>
> +  Â  Â if (openpty(&pty_master, &pty_slave, pty_name, NULL, NULL) < 0) {<br>
> +  Â  Â  Â  Â  Â  Â fprintf(fp, "ERROR: openpty() failed with: %s.\n", strerror(errno));<br>
> +  Â  Â  Â  Â  Â  Â return -1;<br>
> +  Â  Â }<br>
> +<br>
> +  Â  Â if ((gptr = getgrnam(pty_name)) != 0) {<br>
> +  Â  Â  Â  Â  Â  Â gid = gptr->gr_gid;<br>
> +  Â  Â } else {<br>
> +  Â  Â  Â  Â  Â  Â /* If the tty group does not exist, don't change the<br>
> +  Â  Â  Â  Â  Â  Â  * group on the slave pty, only the owner<br>
> +  Â  Â  Â  Â  Â  Â  */<br>
> +  Â  Â  Â  Â  Â  Â gid = -1;<br>
> +  Â  Â }<br>
> +<br>
> +  Â  Â /* chown/chmod the corresponding pty, if possible.<br>
> +  Â  Â  * This will only work if the process has root permissions.<br>
> +  Â  Â  */<br>
> +  Â  Â if (chown(pty_name, getuid(), gid) != 0) {<br>
> +  Â  Â  Â  Â  Â  Â fprintf(fp, "ERROR; chown() failed with: %s.\n", strerror(errno));<br>
> +  Â  Â }<br>
> +<br>
> +  Â  Â /* Makes the slave read/writeable for the user. */<br>
> +  Â  Â if (chmod(pty_name, S_IRUSR|S_IWUSR) != 0) {<br>
> +  Â  Â  Â  Â  Â  Â fprintf(fp, "ERROR: chmod() failed with: %s.\n", strerror(errno));<br>
> +  Â  Â }<br>
> +<br>
> +  Â  Â if ((slave = open(pty_name, O_RDWR)) == -1) {<br>
> +  Â  Â  Â  Â  Â  Â fprintf(fp, "ERROR: open() failed with: %s.\n", strerror(errno));<br>
> +  Â  Â }<br>
> +  Â  Â return (slave);<br>
> +}<br>
> +<br>
> +<br>
>  Â int<br>
>  Â run_ptests(struct ptest_list *head, const struct ptest_options opts,<br>
>  Â  Â  Â  Â  Â  Â  Â const char *progname, FILE *fp, FILE *fp_stderr)<br>
> @@ -362,6 +414,8 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,<br>
>  Â  Â  Â int timeouted;<br>
>  Â  Â  Â time_t sttime, entime;<br>
>  Â  Â  Â int duration;<br>
> +  Â  Â int slave;<br>
> +  Â  Â int pgid = -1;<br>
>  Â <br>
>  Â  Â  Â if (opts.xml_filename) {<br>
>  Â  Â  Â  Â  Â  Â  Â xh = xml_create(ptest_list_length(head), opts.xml_filename);<br>
> @@ -379,7 +433,6 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â close(pipefd_stdout[1]);<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â break;<br>
>  Â  Â  Â  Â  Â  Â  Â }<br>
> -<br>
>  Â  Â  Â  Â  Â  Â  Â fprintf(fp, "START: %s\n", progname);<br>
>  Â  Â  Â  Â  Â  Â  Â PTEST_LIST_ITERATE_START(head, p);<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â char *ptest_dir = strdup(p->run_ptest);<br>
> @@ -388,6 +441,13 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â break;<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â }<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â dirname(ptest_dir);<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â if (ioctl(0, TIOCNOTTY) == -1) {<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â fprintf(fp, "ERROR: Unable to detach from controlling tty, %s\n", strerror(errno));<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â }<br>
> +<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â if ((pgid = getpgid(0)) == -1) {<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â fprintf(fp, "ERROR: getpgid() failed, %s\n", strerror(errno));<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â }<br>
>  Â <br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â child = fork();<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â if (child == -1) {<br>
> @@ -395,13 +455,33 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â rc = -1;<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â break;<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â } else if (child == 0) {<br>
> -  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â setsid();<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â close(0);<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â if ((slave = setup_slave_pty(fp)) < 0) {<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â fprintf(fp, "ERROR: could not setup pty (%d).", slave);<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â }<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â if (setpgid(0,pgid) == -1) {<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â fprintf(fp, "ERROR: setpgid() failed, %s\n", strerror(errno));<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â }<br>
> +<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â if (setsid() ==  -1) {<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â fprintf(fp, "ERROR: setsid() failed, %s\n", strerror(errno));<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â }<br>
> +<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â if (ioctl(0, TIOCSCTTY, NULL) == -1) {<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â fprintf(fp, "ERROR: Unable to attach to controlling tty, %s\n", strerror(errno));<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â }<br>
> +<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â run_child(p->run_ptest, pipefd_stdout[1], pipefd_stderr[1]);<br>
> +<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â } else {<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â int status;<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â int fds[2]; fds[0] = pipefd_stdout[0]; fds[1] = pipefd_stderr[0];<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â FILE *fps[2]; fps[0] = fp; fps[1] = fp_stderr;<br>
>  Â <br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â if (setpgid(child, pgid) == -1) {<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â fprintf(fp, "ERROR: setpgid() failed, %s\n", strerror(errno));<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â }<br>
> +<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â sttime = time(NULL);<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, sttime));<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â fprintf(fp, "BEGIN: %s\n", ptest_dir);<br>
> <br>
<br>
<br>
-- <br>
# Randy MacLeod<br>
# Wind River Linux<br>
</blockquote></div></div>