[yocto] [ptest-runner][PATCH v2 4/4] utils: ensure child can be session leader
Anibal Limon
anibal.limon at linaro.org
Thu Jun 27 08:27:05 PDT 2019
On Wed, 26 Jun 2019 at 10:56, Randy MacLeod <randy.macleod at 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 at windriver.com
> > <mailto:randy.macleod at 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 at windriver.com
> > <mailto:sakib.sajal at windriver.com>>
> > > Signed-off-by: Randy MacLeod <Randy.MacLeod at windriver.com
> > <mailto:Randy.MacLeod at 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.yoctoproject.org/pipermail/yocto/attachments/20190627/f333fc1c/attachment-0001.html>
More information about the yocto
mailing list