[yocto] [ptest-runner][PATCH v2 4/4] utils: ensure child can be session leader

Randy MacLeod randy.macleod at windriver.com
Wed Jun 19 10:49:43 PDT 2019


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 at windriver.com>
> Signed-off-by: Randy MacLeod <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


More information about the yocto mailing list