On Wed, 26 Jun 2019 at 10:56, Randy MacLeod <randy.macl...@windriver.com> wrote:
> On 6/25/19 9:51 PM, Anibal Limon wrote: > > > > > > On Wed, 19 Jun 2019 at 12:50, Randy MacLeod <randy.macl...@windriver.com > > <mailto:randy.macl...@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.sa...@windriver.com > > <mailto:sakib.sa...@windriver.com>> > > > Signed-off-by: Randy MacLeod <randy.macl...@windriver.com > > <mailto:randy.macl...@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 >
-- _______________________________________________ yocto mailing list yocto@yoctoproject.org https://lists.yoctoproject.org/listinfo/yocto
