All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] waitpid: new API (part 2)
@ 2016-08-10  8:40 Stanislav Kholmanskikh
  2016-08-10  8:40 ` [LTP] [PATCH 1/8] waitpid09: use the new API Stanislav Kholmanskikh
  0 siblings, 1 reply; 31+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-10  8:40 UTC (permalink / raw)
  To: ltp

Hi!

This is the second (and last) part of changes to syscalls/waitpid test cases
with the final aim to substitute wait_for_parent() with the checkpoint interface.

waitpid09 was updated in accordance with

http://lists.linux.it/pipermail/ltp/2016-July/002238.html

In particular:
 * the code from do_exit() was moved to the 'if' block
 * used tst_strerrno() to print set and expected errno values

As for waitpid12. In contrast with

http://lists.linux.it/pipermail/ltp/2016-July/002218.html

I decided to keep the process group logic in order to have uniform coverage:

waitpid06: waitpid(-1, , 0)
waitpid07: waitpid(-1, , WNOHANG)
waitpid08: waitpid(-1, , WUNTRACED)
waitpid11: waitpid(0, , 0), waitpid(-group, , 0)
waitpid12: waitpid(0, , WNOHANG), waitpid(-group, , WNOHANG)
waitpid13: waitpid(0, , WUNTRACED), waitpid(-group, , WUNTRACED)

All other patches are new.

Thanks.
 


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

* [LTP] [PATCH 1/8] waitpid09: use the new API
  2016-08-10  8:40 [LTP] waitpid: new API (part 2) Stanislav Kholmanskikh
@ 2016-08-10  8:40 ` Stanislav Kholmanskikh
  2016-08-10  8:41   ` [LTP] [PATCH 2/8] waitpid10: " Stanislav Kholmanskikh
  2016-08-15 13:55   ` [LTP] [PATCH 1/8] waitpid09: " Cyril Hrubis
  0 siblings, 2 replies; 31+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-10  8:40 UTC (permalink / raw)
  To: ltp

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/syscalls/waitpid/waitpid09.c |  438 ++++++++++---------------
 1 files changed, 176 insertions(+), 262 deletions(-)

diff --git a/testcases/kernel/syscalls/waitpid/waitpid09.c b/testcases/kernel/syscalls/waitpid/waitpid09.c
index 1339c82..2d74626 100644
--- a/testcases/kernel/syscalls/waitpid/waitpid09.c
+++ b/testcases/kernel/syscalls/waitpid/waitpid09.c
@@ -1,57 +1,37 @@
 /*
+ * Copyright (c) International Business Machines  Corp., 2001
  *
- *   Copyright (c) International Business Machines  Corp., 2001
+ * This program is free software;  you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
  *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY;  without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+ * the GNU General Public License for more details.
  *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-/*
- * NAME
- *	waitpid09.c
- *
- * DESCRIPTION
- *	Check ability of parent to wait until child returns, and that the
- *	child's process id is returned through the waitpid. Check that
- *	waitpid returns immediately if no child is present.
- *
- * ALGORITHM
- *	case 0:
- *		Parent forks a child and waits. Parent should do nothing
- *		further until child returns. The pid of the forked child
- *		should match the returned value from the waitpid.
- *
- *	case 1:
- *		Parent calls a waitpid with no children waiting. Waitpid
- *		should return a -1 since there are no children to wait for.
- *
- * USAGE:  <for command-line>
- *      waitpid09 [-c n] [-e] [-i n] [-I x] [-P x] [-t]
- *      where,  -c n : Run n copies concurrently.
- *              -e   : Turn on errno logging.
- *              -i n : Execute test n times.
- *              -I x : Execute test for x seconds.
- *              -P x : Pause for x seconds between iterations.
- *              -t   : Turn on syscall timing.
+ * You should have received a copy of the GNU General Public License
+ * along with this program.
  *
  * History
  *	07/2001 John George
  *		-Ported
  *      04/2002 wjhuie sigset cleanups
- *
- * Restrictions
- *	None
+ */
+
+/*
+ * case 0:
+ *        waitpid(pid, WNOHANG) should return 0 if there is a running child
+ * case 1:
+ *        waitpid(pid, WNOHANG) should return the pid of the child if
+ *        the child has exited
+ * case 2:
+ *        waitpid(-1, 0) should return -1 with ECHILD if
+ *        there are no children to wait for.
+ * case 3:
+ *        waitpid(-1, WNOHANG) should return -1 with ECHILD if
+ *        there are no children to wait for.
  */
 
 #define _GNU_SOURCE 1
@@ -60,243 +40,177 @@
 #include <errno.h>
 #include <sys/wait.h>
 #include <stdlib.h>
+#include "tst_test.h"
 
-#include "test.h"
+static void cleanup_pid(pid_t pid)
+{
+	if (pid > 0) {
+		kill(pid, SIGKILL);
+		waitpid(pid, NULL, 0);
+	}
+}
 
-char *TCID = "waitpid09";
-int TST_TOTAL = 1;
-volatile int intintr;
+static void case0(void)
+{
+	pid_t pid, ret;
+	int status;
 
-static void setup(void);
-static void cleanup(void);
-static void inthandlr();
-static void do_exit(void);
-static void setup_sigint(void);
-#ifdef UCLINUX
-static void do_exit_uclinux(void);
-#endif
+	pid = SAFE_FORK();
+	if (pid == 0) {
+		TST_CHECKPOINT_WAIT(0);
 
-int main(int argc, char **argv)
-{
-	int lc;
-
-	int fail, pid, status, ret;
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-#ifdef UCLINUX
-	maybe_run_child(&do_exit_uclinux, "");
-#endif
-
-	setup();
-
-	pid = FORK_OR_VFORK();
-	if (pid < 0) {
-		tst_brkm(TFAIL, cleanup, "Fork Failed");
-	} else if (pid == 0) {
-		/*
-		 * Child:
-		 * Set up to catch SIGINT.  The kids will wait till a
-		 * SIGINT has been received before they proceed.
-		 */
-		setup_sigint();
-
-		/* check for looping state if -i option is given */
-		for (lc = 0; TEST_LOOPING(lc); lc++) {
-			/* reset tst_count in case we are looping */
-			tst_count = 0;
-
-			intintr = 0;
-
-			fail = 0;
-			pid = FORK_OR_VFORK();
-			if (pid < 0) {
-				tst_brkm(TFAIL, cleanup, "Fork failed.");
-			} else if (pid == 0) {	/* child */
-#ifdef UCLINUX
-				if (self_exec(argv[0], "") < 0) {
-					tst_brkm(TFAIL, cleanup,
-						 "self_exec failed");
-				}
-#else
-				do_exit();
-#endif
-			} else {	/* parent */
-
-				/*
-				 *Check that waitpid with WNOHANG returns zero
-				 */
-				while (((ret = waitpid(pid, &status, WNOHANG))
-					!= 0) || (errno == EINTR)) {
-					if (ret == -1)
-						continue;
-
-					tst_resm(TFAIL, "return value for "
-						 "WNOHANG expected 0 got %d",
-						 ret);
-					fail = 1;
-				}
-#ifdef UCLINUX
-				/* Give the kids a chance to setup SIGINT again, since
-				 * this is cleared by exec().
-				 */
-				sleep(3);
-#endif
-
-				/* send SIGINT to child to tell it to proceed */
-				if (kill(pid, SIGINT) < 0) {
-					tst_resm(TFAIL, "Kill of child failed, "
-						 "errno = %d", errno);
-					fail = 1;
-				}
-
-				while (((ret = waitpid(pid, &status, 0)) != -1)
-				       || (errno == EINTR)) {
-					if (ret == -1)
-						continue;
-
-					if (ret != pid) {
-						tst_resm(TFAIL, "Expected %d "
-							 "got %d as proc id of "
-							 "child", pid, ret);
-						fail = 1;
-					}
-
-					if (status != 0) {
-						tst_resm(TFAIL, "status value "
-							 "got %d expected 0",
-							 status);
-						fail = 1;
-					}
-				}
-			}
-
-			pid = FORK_OR_VFORK();
-			if (pid < 0) {
-				tst_brkm(TFAIL, cleanup, "Second fork failed.");
-			} else if (pid == 0) {	/* child */
-				exit(0);
-			} else {	/* parent */
-				/* Give the child time to startup and exit */
-				sleep(2);
-
-				while (((ret = waitpid(pid, &status, WNOHANG))
-					!= -1) || (errno == EINTR)) {
-					if (ret == -1)
-						continue;
-
-					if (ret != pid) {
-						tst_resm(TFAIL, "proc id %d "
-							 "and retval %d do not "
-							 "match", pid, ret);
-						fail = 1;
-					}
-
-					if (status != 0) {
-						tst_resm(TFAIL, "non zero "
-							 "status received %d",
-							 status);
-						fail = 1;
-					}
-				}
-			}
-
-			if (fail)
-				tst_resm(TFAIL, "case 1 FAILED");
-			else
-				tst_resm(TPASS, "case 1 PASSED");
-
-			fail = 0;
-			ret = waitpid(pid, &status, 0);
-
-			if (ret != -1) {
-				tst_resm(TFAIL, "Expected -1 got %d", ret);
-				fail = 1;
-			}
-			if (errno != ECHILD) {
-				tst_resm(TFAIL, "Expected ECHILD got %d",
-					 errno);
-				fail = 1;
-			}
-
-			ret = waitpid(pid, &status, WNOHANG);
-			if (ret != -1) {
-				tst_resm(TFAIL, "WNOHANG: Expected -1 got %d",
-					 ret);
-				fail = 1;
-			}
-			if (errno != ECHILD) {
-				tst_resm(TFAIL, "WNOHANG: Expected ECHILD got "
-					 "%d", errno);
-				fail = 1;
-			}
-
-			if (fail)
-				tst_resm(TFAIL, "case 2 FAILED");
-			else
-				tst_resm(TPASS, "case 2 PASSED");
-		}
-
-		cleanup();
-	} else {
-		/* wait for the child to return */
-		waitpid(pid, &status, 0);
-		if (WEXITSTATUS(status) != 0) {
-			tst_brkm(TBROK, cleanup, "child returned bad "
-				 "status");
-		}
+		exit(0);
 	}
 
-	tst_exit();
-}
+	for (;;) {
+		ret = waitpid(pid, &status, WNOHANG);
 
-/*
- * setup_sigint()
- *	sets up a SIGINT handler
- */
-static void setup_sigint(void)
-{
-	if ((sig_t) signal(SIGINT, inthandlr) == SIG_ERR) {
-		tst_brkm(TFAIL, cleanup, "signal SIGINT failed, errno = %d",
-			 errno);
+		if ((ret == -1) && (errno == EINTR))
+			continue;
+
+		if (ret == 0)
+			break;
+
+		tst_res(TFAIL, "waitpid(WNOHANG) returned %d, expected 0",
+			ret);
+		cleanup_pid(pid);
+		return;
 	}
-}
 
-static void setup(void)
-{
-	TEST_PAUSE;
-}
+	TST_CHECKPOINT_WAKE(0);
 
-static void cleanup(void)
-{
+	for (;;) {
+		ret = waitpid(pid, &status, 0);
+
+		if ((ret == -1) && (errno == EINTR))
+			continue;
+
+		if (ret == pid)
+			break;
+
+		tst_res(TFAIL, "waitpid(0) returned %d, expected %d",
+			 ret, pid);
+		cleanup_pid(pid);
+		return;
+	}
+
+	if (!WIFEXITED(status)) {
+		tst_res(TFAIL, "Child exited abnormally");
+		return;
+	}
+
+	if (WEXITSTATUS(status) != 0) {
+		tst_res(TFAIL, "Child exited with %d, expected 0",
+			WEXITSTATUS(status));
+		return;
+	}
+
+	tst_res(TPASS, "Case 0 PASSED");
 }
 
-static void inthandlr(void)
+static void case1(void)
 {
-	intintr++;
+	pid_t pid, ret;
+	int status;
+
+	pid = SAFE_FORK();
+	if (pid == 0)
+		exit(0);
+
+	for (;;) {
+		ret = waitpid(pid, &status, WNOHANG);
+
+		if ((ret == -1) && (errno == EINTR))
+			continue;
+		if (ret == 0)
+			continue;
+
+		if (ret == pid)
+			break;
+
+		tst_res(TFAIL, "waitpid(WNOHANG) returned %d, expected %d",
+			ret, pid);
+		cleanup_pid(pid);
+		return;
+	}
+
+	if (!WIFEXITED(status)) {
+		tst_res(TFAIL, "Child exited abnormally");
+		return;
+	}
+
+	if (WEXITSTATUS(status) != 0) {
+		tst_res(TFAIL, "Child exited with %d, expected 0",
+			WEXITSTATUS(status));
+		return;
+	}
+
+	tst_res(TPASS, "Case 1 PASSED");
 }
 
-static void wait_for_parent(void)
+static void case2(void)
 {
-	int testvar;
-	while (!intintr)
-		testvar = 0;
+	pid_t ret;
+	int status;
+
+	ret = waitpid(-1, &status, 0);
+
+	if (ret != -1) {
+		tst_res(TFAIL, "Expected -1, got %d", ret);
+		return;
+	}
+	if (errno != ECHILD) {
+		tst_res(TFAIL, "Expected %s, got %s",
+			tst_strerrno(ECHILD), tst_strerrno(errno));
+		return;
+	}
+
+	tst_res(TPASS, "Case 2 PASSED");
 }
 
-static void do_exit(void)
+static void case3(void)
 {
-	wait_for_parent();
-	exit(0);
+	pid_t ret;
+	int status;
+
+	ret = waitpid(-1, &status, WNOHANG);
+	if (ret != -1) {
+		tst_res(TFAIL, "WNOHANG: Expected -1, got %d", ret);
+		return;
+	}
+	if (errno != ECHILD) {
+		tst_res(TFAIL, "WNOHANG: Expected %s, got %s",
+			tst_strerrno(ECHILD), tst_strerrno(errno));
+		return;
+	}
+
+	tst_res(TPASS, "Case 3 PASSED");
 }
 
-#ifdef UCLINUX
-/*
- * do_exit_uclinux()
- *	Sets up SIGINT handler again, then calls do_exit
- */
-static void do_exit_uclinux(void)
+static void waitpid09_test(unsigned int id)
 {
-	setup_sigint();
-	do_exit();
+	switch (id) {
+	case 0:
+		case0();
+		break;
+	case 1:
+		case1();
+		break;
+	case 2:
+		case2();
+		break;
+	case 3:
+		case3();
+		break;
+	default:
+		tst_brk(TBROK, "Unknown %d", id);
+	}
 }
-#endif
+
+static struct tst_test test = {
+	.tid = "waitpid09",
+	.forks_child = 1,
+	.needs_checkpoints = 1,
+	.test = waitpid09_test,
+	.tcnt = 4,
+};
-- 
1.7.1


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

* [LTP] [PATCH 2/8] waitpid10: use the new API
  2016-08-10  8:40 ` [LTP] [PATCH 1/8] waitpid09: use the new API Stanislav Kholmanskikh
@ 2016-08-10  8:41   ` Stanislav Kholmanskikh
  2016-08-10  8:41     ` [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test() Stanislav Kholmanskikh
  2016-08-15 14:36     ` [LTP] [PATCH 2/8] waitpid10: use the new API Cyril Hrubis
  2016-08-15 13:55   ` [LTP] [PATCH 1/8] waitpid09: " Cyril Hrubis
  1 sibling, 2 replies; 31+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-10  8:41 UTC (permalink / raw)
  To: ltp

* update the description to match the actions performed in the test
* make use of the new API
* make use of reap_children() in do_fork()

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/syscalls/waitpid/waitpid10.c |  312 ++++---------------------
 1 files changed, 47 insertions(+), 265 deletions(-)

diff --git a/testcases/kernel/syscalls/waitpid/waitpid10.c b/testcases/kernel/syscalls/waitpid/waitpid10.c
index d239fda..a505ade 100644
--- a/testcases/kernel/syscalls/waitpid/waitpid10.c
+++ b/testcases/kernel/syscalls/waitpid/waitpid10.c
@@ -14,263 +14,59 @@
  * the GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
- * along with this program;  if not, write to the Free Software Foundation,
- * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ * along with this program.
  */
 
 /*
- * Tests to see if pid's returned from fork and waitpid are same
+ * Tests to see if pids returned from fork and waitpid are same
  *
- * Set up to catch SIGINTs, SIGALRMs, and the real time timer. Until the timer
- * interrupts, do the following. Fork 8 kids, 2 will immediately exit, 2 will
- * sleep, 2 will be compute bound, and 2 will fork another child, both which
- * will do mkdirs on the same directory 50 times. When the timer expires, kill
- * all kids and remove the directory.
+ * Fork 8 kids, 2 will immediately exit, 2 will sleep, 2 will be compute bound
+ * and 2 will fork/reap a child for 50 times.
  */
 
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <sys/wait.h>
+#include "waitpid_common.h"
 
-#include <stdio.h>
-#include <signal.h>
-#include <errno.h>
-#include "test.h"
-
-#define	MAXKIDS	8
-
-char *TCID = "waitpid10";
-int TST_TOTAL = 1;
-
-volatile int intintr;
-
-static void setup(void);
-static void cleanup(void);
-static void inthandlr();
-static void wait_for_parent(void);
-static void do_exit(void);
 static void do_compute(void);
 static void do_fork(void);
 static void do_sleep(void);
 
-static int fail;
-static int fork_kid_pid[MAXKIDS];
-
-#ifdef UCLINUX
-static char *argv0;
-#endif
-
-int main(int ac, char **av)
+static void do_child_1(void)
 {
-	int kid_count, ret_val, status, nkids;
-	int i, j, k, found;
-	int wait_kid_pid[MAXKIDS];
-
-	int lc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-#ifdef UCLINUX
-	argv0 = av[0];
-
-	maybe_run_child(&do_exit, "n", 1);
-	maybe_run_child(&do_compute, "n", 2);
-	maybe_run_child(&do_fork, "n", 3);
-	maybe_run_child(&do_sleep, "n", 4);
-#endif
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		fail = 0;
-		kid_count = 0;
-		intintr = 0;
-		for (i = 0; i < MAXKIDS; i++) {
-			ret_val = FORK_OR_VFORK();
-			if (ret_val < 0)
-				tst_brkm(TBROK|TERRNO, cleanup,
-					 "Fork kid %d failed.", i);
-
-			if (ret_val == 0) {
-				if (i == 0 || i == 1) {
-#ifdef UCLINUX
-					if (self_exec(argv0, "n", 1) < 0)
-						tst_brkm(TBROK|TERRNO, cleanup,
-							 "self_exec %d failed",
-							 i);
-#else
-					do_exit();
-#endif
-				}
-
-				if (i == 2 || i == 3) {
-#ifdef UCLINUX
-					if (self_exec(argv0, "n", 2) < 0)
-						tst_brkm(TBROK|TERRNO, cleanup,
-							 "self_exec %d failed",
-							 i);
-#else
-					do_compute();
-#endif
-				}
-
-				if (i == 4 || i == 5) {
-#ifdef UCLINUX
-					if (self_exec(argv0, "n", 3) < 0)
-						tst_brkm(TBROK|TERRNO, cleanup,
-							 "self_exec %d failed",
-							 i);
-#else
-					do_fork();
-#endif
-				}
-
-				if (i == 6 || i == 7) {
-#ifdef UCLINUX
-					if (self_exec(argv0, "n", 4) < 0)
-						tst_brkm(TBROK|TERRNO, cleanup,
-							 "self_exec %d failed",
-							 i);
-#else
-					do_sleep();
-#endif
-
-				}
-
-			}
-
-			fork_kid_pid[kid_count++] = ret_val;
-		}
-
-		nkids = kid_count;
-
-		/*
-		 * Now send all the kids a SIGUSR1 to tell them to
-		 * proceed. We sleep for a while first to allow the
-		 * children to initialize their "intintr" variables
-		 * and get set up.
-		 */
-		sleep(15);
+	pid_t pid;
+	int i;
 
-		for (i = 0; i < nkids; i++) {
-			if (kill(fork_kid_pid[i], SIGUSR1) < 0) {
-				tst_brkm(TBROK|TERRNO, cleanup, "Kill of child "
-						"%d failed", i);
-			}
-		}
+	for (i = 0; i < MAXKIDS; i++) {
+		pid = SAFE_FORK();
+		if (pid == 0) {
+			if (i == 0 || i == 1)
+				do_exit(0);
 
-		/* Wait till all kids have terminated. */
-		kid_count = 0;
-		errno = 0;
-		for (i = 0; i < nkids; i++) {
-			while (((ret_val = waitpid(fork_kid_pid[i],
-							&status, 0)) != -1)
-					|| (errno == EINTR)) {
-				if (ret_val == -1)
-					continue;
+			if (i == 2 || i == 3)
+				do_compute();
 
-				wait_kid_pid[kid_count++] = ret_val;
-			}
-		}
+			if (i == 4 || i == 5)
+				do_fork();
 
-		/*
-		 * Check that for every entry in the fork_kid_pid
-		 * array, there is a matching pid in the
-		 * wait_kid_pid array.
-		 */
-		for (i = 0; i < MAXKIDS; i++) {
-			found = 0;
-			for (j = 0; j < MAXKIDS; j++) {
-				if (fork_kid_pid[i] == wait_kid_pid[j]) {
-					found = 1;
-					break;
-				}
-			}
-			if (!found) {
-				tst_resm(TFAIL, "Did not find a "
-						"wait_kid_pid for the "
-						"fork_kid_pid of %d",
-						fork_kid_pid[i]);
-				for (k = 0; k < nkids; k++) {
-					tst_resm(TFAIL,
-							"fork_kid_pid[%d] = "
-							"%d", k,
-							fork_kid_pid[k]);
-				}
-				for (k = 0; k < kid_count; k++) {
-					tst_resm(TFAIL,
-							"wait_kid_pid[%d] = "
-							"%d", k,
-							wait_kid_pid[k]);
-				}
-				fail = 1;
-			}
+			if (i == 6 || i == 7)
+				do_sleep();
 		}
 
-		memset(fork_kid_pid, 0, sizeof(fork_kid_pid));
-
-		if (fail)
-			tst_resm(TFAIL, "Test FAILED");
-		else
-			tst_resm(TPASS, "Test PASSED");
+		fork_kid_pid[i] = pid;
 	}
 
-	cleanup();
-	tst_exit();
-}
-
-static void setup(void)
-{
-	struct sigaction act;
-
-	tst_sig(FORK, DEF_HANDLER, cleanup);
+	TST_CHECKPOINT_WAKE2(0, MAXKIDS);
 
-	TEST_PAUSE;
+	if (reap_children(0, 0, fork_kid_pid, MAXKIDS))
+		return;
 
-	act.sa_handler = inthandlr;
-	act.sa_flags = SA_RESTART;
-	sigemptyset(&act.sa_mask);
-
-	if (sigaction(SIGUSR1, &act, NULL) < 0)
-		tst_brkm(TBROK|TERRNO, cleanup,
-			 "sigaction(SIGUSR1, ...) failed");
-
-	intintr = 0;
-
-}
-
-static void cleanup(void)
-{
-	int i;
-
-	for (i = 0; i < MAXKIDS; i++) {
-		if (fork_kid_pid[i] > 0)
-			kill(fork_kid_pid[i], SIGKILL);
-	}
-}
-
-static void inthandlr(void)
-{
-	intintr++;
-}
-
-static void wait_for_parent(void)
-{
-	while (!intintr)
-		usleep(100);
-}
-
-static void do_exit(void)
-{
-	wait_for_parent();
-	exit(3);
+	tst_res(TPASS, "Test PASSED");
 }
 
 static void do_compute(void)
 {
 	int i;
 
-	wait_for_parent();
+	TST_CHECKPOINT_WAIT(0);
 
 	for (i = 0; i < 100000; i++) ;
 	for (i = 0; i < 100000; i++) ;
@@ -283,57 +79,43 @@ static void do_compute(void)
 	for (i = 0; i < 100000; i++) ;
 	for (i = 0; i < 100000; i++) ;
 
-	exit(4);
+	exit(3);
 }
 
 static void do_fork(void)
 {
-	int fork_pid, wait_pid;
-	int status, i;
+	pid_t fork_pid;
+	int i;
 
-	wait_for_parent();
+	TST_CHECKPOINT_WAIT(0);
 
 	for (i = 0; i < 50; i++) {
-		fork_pid = FORK_OR_VFORK();
-		if (fork_pid < 0) {
-			tst_brkm(TBROK|TERRNO, NULL, "Fork failed");
-		}
-		if (fork_pid == 0) {
-#ifdef UCLINUX
-			if (self_exec(argv0, "n", 1) < 0) {
-				tst_brkm(TFAIL, NULL,
-					 "do_fork self_exec failed");
-			}
-#else
-			do_exit();
-#endif
-		}
-
-		errno = 0;
-		while (((wait_pid = waitpid(fork_pid, &status, 0)) != -1) ||
-		       (errno == EINTR)) {
-			if (wait_pid == -1)
-				continue;
+		fork_pid = SAFE_FORK();
+		if (fork_pid == 0)
+			exit(3);
 
-			if (fork_pid != wait_pid) {
-				tst_resm(TFAIL, "Didnt get a pid returned "
-					 "from waitpid that matches the one "
-					 "returned by fork");
-				tst_resm(TFAIL, "fork pid = %d, wait pid = "
-					 "%d", fork_pid, wait_pid);
-				fail = 1;
-			}
-		}
+		if (reap_children(fork_pid, 0, &fork_pid, 1))
+			break;
 	}
 
-	exit(4);
+	exit(3);
 }
 
 static void do_sleep(void)
 {
-	wait_for_parent();
+	TST_CHECKPOINT_WAIT(0);
+
 	sleep(1);
 	sleep(1);
 
-	exit(4);
+	exit(3);
 }
+
+static struct tst_test test = {
+	.tid = "waitpid10",
+	.forks_child = 1,
+	.needs_checkpoints = 1,
+	.setup = waitpid_setup,
+	.cleanup = waitpid_cleanup,
+	.test_all = waitpid_test,
+};
-- 
1.7.1


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

* [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test()
  2016-08-10  8:41   ` [LTP] [PATCH 2/8] waitpid10: " Stanislav Kholmanskikh
@ 2016-08-10  8:41     ` Stanislav Kholmanskikh
  2016-08-10  8:41       ` [LTP] [PATCH 4/8] syscalls/waitpid: make reap_children() fail if errno is not ECHILD Stanislav Kholmanskikh
  2016-08-15 15:27       ` [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test() Cyril Hrubis
  2016-08-15 14:36     ` [LTP] [PATCH 2/8] waitpid10: use the new API Cyril Hrubis
  1 sibling, 2 replies; 31+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-10  8:41 UTC (permalink / raw)
  To: ltp

Several test cases call waitpid() and then verify the returned
value and errno set. Moved this code into a function.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/syscalls/waitpid/waitpid07.c      |    6 +---
 testcases/kernel/syscalls/waitpid/waitpid08.c      |   24 +---------------
 testcases/kernel/syscalls/waitpid/waitpid_common.h |   29 ++++++++++++++++++++
 3 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/testcases/kernel/syscalls/waitpid/waitpid07.c b/testcases/kernel/syscalls/waitpid/waitpid07.c
index 74da914..89fcbac 100644
--- a/testcases/kernel/syscalls/waitpid/waitpid07.c
+++ b/testcases/kernel/syscalls/waitpid/waitpid07.c
@@ -48,12 +48,8 @@ static void do_child_1(void)
 	}
 
 	/* Check that waitpid with WNOHANG returns zero */
-	pid = waitpid(-1, &status, WNOHANG);
-	if (pid != 0) {
-		tst_res(TFAIL, "waitpid() returned %d, expected 0",
-			pid);
+	if (waitpid_ret_test(-1, &status, WNOHANG, 0, 0))
 		return;
-	}
 
 	TST_CHECKPOINT_WAKE2(0, MAXKIDS);
 
diff --git a/testcases/kernel/syscalls/waitpid/waitpid08.c b/testcases/kernel/syscalls/waitpid/waitpid08.c
index 0f0ee9d..e17663f 100644
--- a/testcases/kernel/syscalls/waitpid/waitpid08.c
+++ b/testcases/kernel/syscalls/waitpid/waitpid08.c
@@ -51,36 +51,14 @@ static void do_child_1(void)
 	/* Check that waitpid with WNOHANG|WUNTRACED returns
 	 * zero
 	 */
-	pid = waitpid(-1, &status, WNOHANG | WUNTRACED);
-	if (pid != 0) {
-		tst_res(TFAIL,
-			"waitpid(WNOHANG | WUNTRACED) returned %d, expected 0",
-			pid);
+	if (waitpid_ret_test(-1, &status, WNOHANG | WUNTRACED, 0, 0))
 		return;
-	}
 
 	TST_CHECKPOINT_WAKE2(0, MAXKIDS);
 
 	if (reap_children(-1, WUNTRACED, fork_kid_pid, MAXKIDS))
 		return;
 
-	/*
-	 * Check that waitpid(WUNTRACED) returns -1 when no
-	 * stopped children
-	 */
-	pid = waitpid(-1, &status, WUNTRACED);
-	if (pid != -1) {
-		tst_res(TFAIL, "waitpid(WUNTRACED) returned %d, expected -1",
-			pid);
-		return;
-	}
-
-	if (errno != ECHILD) {
-		tst_res(TFAIL, "waitpid(WUNTRACED) set errno %s, expected %s",
-			tst_strerrno(errno), tst_strerrno(ECHILD));
-		return;
-	}
-
 	tst_res(TPASS, "Test PASSED");
 }
 
diff --git a/testcases/kernel/syscalls/waitpid/waitpid_common.h b/testcases/kernel/syscalls/waitpid/waitpid_common.h
index 008577a..df0de9b 100644
--- a/testcases/kernel/syscalls/waitpid/waitpid_common.h
+++ b/testcases/kernel/syscalls/waitpid/waitpid_common.h
@@ -75,6 +75,35 @@ static void do_exit(int stop)
 	exit(3);
 }
 
+static int waitpid_errno_check(int err, int exp_err)
+{
+	if (err != exp_err) {
+		tst_res(TFAIL, "waitpid() set errno to %s, expected %s",
+			tst_strerrno(err), tst_strerrno(exp_err));
+		return -1;
+	}
+
+	return 0;
+}
+
+int waitpid_ret_test(pid_t wp_pid, int *wp_status, int wp_opts,
+			    pid_t wp_ret, int wp_errno)
+{
+	pid_t ret;
+
+	ret = waitpid(wp_pid, wp_status, wp_opts);
+	if (ret != wp_ret) {
+		tst_res(TFAIL, "waitpid() returned %d, expected %d",
+			ret, wp_ret);
+		return -1;
+	}
+
+	if ((ret == -1) && waitpid_errno_check(errno, wp_errno))
+		return -1;
+
+	return 0;
+}
+
 static int reap_children(pid_t wp_pid, int wp_opts, pid_t *children, int len)
 {
 	pid_t pid;
-- 
1.7.1


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

* [LTP] [PATCH 4/8] syscalls/waitpid: make reap_children() fail if errno is not ECHILD
  2016-08-10  8:41     ` [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test() Stanislav Kholmanskikh
@ 2016-08-10  8:41       ` Stanislav Kholmanskikh
  2016-08-10  8:41         ` [LTP] [PATCH 5/8] waitpid11: update the description Stanislav Kholmanskikh
  2016-08-15 15:49         ` [LTP] [PATCH 4/8] syscalls/waitpid: make reap_children() fail if errno is not ECHILD Cyril Hrubis
  2016-08-15 15:27       ` [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test() Cyril Hrubis
  1 sibling, 2 replies; 31+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-10  8:41 UTC (permalink / raw)
  To: ltp

We expect that waitpid() may fail only with EINTR or ECHILD.
All other errno values signal about an error.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/syscalls/waitpid/waitpid_common.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/testcases/kernel/syscalls/waitpid/waitpid_common.h b/testcases/kernel/syscalls/waitpid/waitpid_common.h
index df0de9b..a2eb508 100644
--- a/testcases/kernel/syscalls/waitpid/waitpid_common.h
+++ b/testcases/kernel/syscalls/waitpid/waitpid_common.h
@@ -117,6 +117,9 @@ static int reap_children(pid_t wp_pid, int wp_opts, pid_t *children, int len)
 			if (errno == EINTR)
 				continue;
 
+			if (waitpid_errno_check(errno, ECHILD))
+				return -1;
+
 			break;
 		}
 
-- 
1.7.1


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

* [LTP] [PATCH 5/8] waitpid11: update the description
  2016-08-10  8:41       ` [LTP] [PATCH 4/8] syscalls/waitpid: make reap_children() fail if errno is not ECHILD Stanislav Kholmanskikh
@ 2016-08-10  8:41         ` Stanislav Kholmanskikh
  2016-08-10  8:41           ` [LTP] [PATCH 6/8] waitpid12: use the new API Stanislav Kholmanskikh
  2016-08-15 15:51           ` [LTP] [PATCH 5/8] waitpid11: update the description Cyril Hrubis
  2016-08-15 15:49         ` [LTP] [PATCH 4/8] syscalls/waitpid: make reap_children() fail if errno is not ECHILD Cyril Hrubis
  1 sibling, 2 replies; 31+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-10  8:41 UTC (permalink / raw)
  To: ltp

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/syscalls/waitpid/waitpid11.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/testcases/kernel/syscalls/waitpid/waitpid11.c b/testcases/kernel/syscalls/waitpid/waitpid11.c
index 9b51e04..ecf1b08 100644
--- a/testcases/kernel/syscalls/waitpid/waitpid11.c
+++ b/testcases/kernel/syscalls/waitpid/waitpid11.c
@@ -25,7 +25,7 @@
  *	Tests to see if pids returned from fork and waitpid are same
  *
  * ALGORITHM
- * 	Check proper functioning of waitpid with pid = -1 and arg = 0
+ *	Check proper functioning of waitpid with pid = 0 and < -1 with arg 0
  */
 
 #include "waitpid_common.h"
-- 
1.7.1


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

* [LTP] [PATCH 6/8] waitpid12: use the new API
  2016-08-10  8:41         ` [LTP] [PATCH 5/8] waitpid11: update the description Stanislav Kholmanskikh
@ 2016-08-10  8:41           ` Stanislav Kholmanskikh
  2016-08-10  8:41             ` [LTP] [PATCH 7/8] waitpid13: " Stanislav Kholmanskikh
  2016-08-16 13:20             ` [LTP] [PATCH 6/8] waitpid12: " Cyril Hrubis
  2016-08-15 15:51           ` [LTP] [PATCH 5/8] waitpid11: update the description Cyril Hrubis
  1 sibling, 2 replies; 31+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-10  8:41 UTC (permalink / raw)
  To: ltp

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/syscalls/waitpid/waitpid12.c |  352 ++++---------------------
 1 files changed, 51 insertions(+), 301 deletions(-)

diff --git a/testcases/kernel/syscalls/waitpid/waitpid12.c b/testcases/kernel/syscalls/waitpid/waitpid12.c
index bf64662..97fc367 100644
--- a/testcases/kernel/syscalls/waitpid/waitpid12.c
+++ b/testcases/kernel/syscalls/waitpid/waitpid12.c
@@ -1,329 +1,79 @@
 /*
+ * Copyright (c) International Business Machines  Corp., 2001
  *
- *   Copyright (c) International Business Machines  Corp., 2001
+ * This program is free software;  you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
  *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY;  without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+ * the GNU General Public License for more details.
  *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-/*
- * NAME
- *	waitpid12.c
- *
- * DESCRIPTION
- *	Tests to see if pid's returned from fork and waitpid are same
- *
- * ALGORITHM
- *	Check proper functioning of waitpid with pid = -1 and arg = WNOHANG
- *
- * USAGE:  <for command-line>
- *      waitpid12 [-c n] [-t]
- *      where,  -c n : Run n copies concurrently.
- *              -t   : Turn on syscall timing.
+ * You should have received a copy of the GNU General Public License
+ * along with this program.
  *
  * History
  *	07/2001 John George
  *		-Ported
  *      04/2002 wjhuie sigset testx for SIG_ERR not < 0, TPASS|TFAIL issued
  *      04/2002 wjhuie sigset cleanups
- *
- * Restrictions
- *	None
  */
 
-#include <sys/types.h>
-#include <signal.h>
-#include <errno.h>
-#include <sys/wait.h>
-#include "test.h"
-
-#define	MAXKIDS	8
-
-char *TCID = "waitpid12";
-int TST_TOTAL = 1;
-
-volatile int intintr;
-static void setup(void);
-static void cleanup(void);
-static void inthandlr();
-static void wait_for_parent(void);
-static void do_exit(void);
-static void setup_sigint(void);
-#ifdef UCLINUX
-static void do_exit_uclinux(void);
-#endif
+/*
+ * DESCRIPTION
+ *	Tests to see if pids returned from fork and waitpid are same
+ *
+ * ALGORITHM
+ *	Check proper functioning of waitpid with pid = 0 and < -1 with arg
+ *	WNOHANG
+ */
 
-static int fail;
+#include "waitpid_common.h"
 
-int main(int argc, char **argv)
+static void do_child_1(void)
 {
-	int kid_count, ret_val, status;
-	int i, j, k, found;
-	int group1, group2;
-	int fork_kid_pid[MAXKIDS], wait_kid_pid[MAXKIDS];
-	int pid;
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-#ifdef UCLINUX
-	maybe_run_child(&do_exit_uclinux, "");
-#endif
-
-	setup();
-
-	tst_count = 0;
-	fail = 0;
-
-	/*
-	 * Need to have test run from child as test driver causes
-	 * test to be a session leader and setpgrp fails.
-	 */
-
-	pid = FORK_OR_VFORK();
-	if (pid > 0) {
-		waitpid(pid, &status, 0);
-		if (WEXITSTATUS(status) != 0) {
-			tst_resm(TFAIL, "child returned bad status");
-			fail = 1;
-		}
-		if (fail)
-			tst_resm(TFAIL, "%s FAILED", TCID);
-		else
-			tst_resm(TPASS, "%s PASSED", TCID);
-		cleanup();
-		tst_exit();
-	} else if (pid < 0) {
-		tst_brkm(TBROK, cleanup, "fork failed");
-	}
-
-	/*
-	 * Set up to catch SIGINT.  The kids will wait till a SIGINT
-	 * has been received before they proceed.
-	 */
-	setup_sigint();
-
-	group1 = getpgrp();
-
-	for (kid_count = 0; kid_count < MAXKIDS; kid_count++) {
-		if (kid_count == (MAXKIDS / 2))
-			group2 = setpgrp();
-
-		intintr = 0;
-		ret_val = FORK_OR_VFORK();
-		if (ret_val == 0) {
-#ifdef UCLINUX
-			if (self_exec(argv[0], "") < 0)
-				tst_resm(TFAIL, "self_exec kid %d "
-					 "failed", kid_count);
-#else
-			do_exit();
-#endif
-		}
-
-		if (ret_val < 0)
-			tst_resm(TFAIL | TERRNO, "forking kid %d failed",
-				 kid_count);
-
-		/* parent */
-		fork_kid_pid[kid_count] = ret_val;
-	}
-
-	/* Check that waitpid with WNOHANG returns zero */
-	ret_val = waitpid(0, &status, WNOHANG);
-	if (ret_val != 0) {
-		tst_resm(TFAIL, "Waitpid returned wrong value");
-		tst_resm(TFAIL, "Expected 0 got %d", ret_val);
-		fail = 1;
-	}
-#ifdef UCLINUX
-	/* Give the kids a chance to setup SIGINT again, since this is
-	 * cleared by exec().
-	 */
-	sleep(3);
-#endif
-
-	/* Now send all the kids a SIGINT to tell them to proceed */
-	for (i = 0; i < MAXKIDS; i++)
-		if (kill(fork_kid_pid[i], SIGINT) < 0)
-			tst_resm(TFAIL | TERRNO, "killing child %d failed", i);
-
-	/*
-	 * Wait till all kids have terminated.  Stash away their
-	 * pid's in an array.
-	 */
-	kid_count = 0;
-	errno = 0;
-	sleep(2);
-	while (((ret_val = waitpid(0, &status, WNOHANG)) != -1) ||
-	       (errno == EINTR)) {
-		if ((ret_val == -1) || (ret_val == 0))
-			continue;
-
-		if (!WIFEXITED(status)) {
-			tst_resm(TFAIL, "Child %d did not exit "
-				 "normally", ret_val);
-			fail = 1;
-		} else {
-			if (WEXITSTATUS(status) != 3) {
-				tst_resm(TFAIL, "Child %d exited with "
-					 "wrong status", ret_val);
-				tst_resm(TFAIL, "Expected 3 got %d",
-					 WEXITSTATUS(status));
-				fail = 1;
-			}
-		}
-		wait_kid_pid[kid_count++] = ret_val;
-	}
-
-	/*
-	 * Check that for every entry in the fork_kid_pid array,
-	 * there is a matching pid in the wait_kid_pid array.  If
-	 * not, it's an error.
-	 */
-	for (i = 0; i < kid_count; i++) {
-		found = 0;
-		for (j = (MAXKIDS / 2); j < MAXKIDS; j++) {
-			if (fork_kid_pid[j] == wait_kid_pid[i]) {
-				found = 1;
-				break;
-			}
-		}
-		if (!found) {
-			tst_resm(TFAIL, "Did not find a wait_kid_pid "
-				 "for the fork_kid_pid of %d", wait_kid_pid[i]);
-			for (k = 0; k < MAXKIDS; k++)
-				tst_resm(TFAIL, "fork_kid_pid[%d] = "
-					 "%d", k, fork_kid_pid[k]);
-			for (k = 0; k < kid_count; k++)
-				tst_resm(TFAIL, "wait_kid_pid[%d] = "
-					 "%d", k, wait_kid_pid[k]);
-			fail = 1;
-		}
-	}
+	pid_t pid, group;
+	int i;
+	int status;
 
-	if (kid_count != (MAXKIDS / 2)) {
-		tst_resm(TFAIL, "Wrong number of children waited on "
-			 "for pid = 0");
-		tst_resm(TFAIL, "Expected 4 got %d", kid_count);
-		fail = 1;
-	}
+	group = SAFE_GETPGID(0);
 
-	/* Make sure can pickup children in a diff. process group */
+	for (i = 0; i < MAXKIDS; i++) {
+		if (i == (MAXKIDS / 2))
+			SAFE_SETPGID(0, 0);
 
-	kid_count = 0;
-	errno = 0;
-	while (((ret_val = waitpid(-(group1), &status, WNOHANG)) !=
-		-1) || (errno == EINTR)) {
-		if (ret_val == -1 || ret_val == 0)
-			continue;
+		pid = SAFE_FORK();
+		if (pid == 0)
+			do_exit(0);
 
-		if (!WIFEXITED(status)) {
-			tst_resm(TFAIL, "Child %d did not exit "
-				 "normally", ret_val);
-			fail = 1;
-		} else {
-			if (WEXITSTATUS(status) != 3) {
-				tst_resm(TFAIL, "Child %d exited with "
-					 "wrong status", ret_val);
-				tst_resm(TFAIL, "Expected 3 got %d",
-					 WEXITSTATUS(status));
-				fail = 1;
-			}
-		}
-		wait_kid_pid[kid_count++] = ret_val;
+		fork_kid_pid[i] = pid;
 	}
 
-	/*
-	 * Check that for every entry in the fork_kid_pid array,
-	 * there is a matching pid in the wait_kid_pid array.  If
-	 * not, it's an error.
-	 */
-	for (i = 0; i < kid_count; i++) {
-		found = 0;
-		for (j = 0; j < (MAXKIDS / 2); j++) {
-			if (fork_kid_pid[j] == wait_kid_pid[i]) {
-				found = 1;
-				break;
-			}
-		}
-		if (!found) {
-			tst_resm(TFAIL, "Did not find a wait_kid_pid "
-				 "for the fork_kid_pid of %d", fork_kid_pid[j]);
-			for (k = 0; k < MAXKIDS; k++)
-				tst_resm(TFAIL, "fork_kid_pid[%d] = "
-					 "%d", k, fork_kid_pid[k]);
-			for (k = 0; k < kid_count; k++)
-				tst_resm(TFAIL, "wait_kid_pid[%d] = "
-					 "%d", k, wait_kid_pid[k]);
-			fail = 1;
-		}
-	}
-	if (kid_count != (MAXKIDS / 2)) {
-		tst_resm(TFAIL, "Wrong number of children waited on "
-			 "for pid = 0");
-		tst_resm(TFAIL, "Expected 4 got %d", kid_count);
-		fail = 1;
-	}
+	if (waitpid_ret_test(0, &status, WNOHANG, 0, 0))
+		return;
 
-	if (fail)
-		tst_resm(TFAIL, "Test FAILED");
-	else
-		tst_resm(TPASS, "Test PASSED");
+	if (waitpid_ret_test(-group, &status, WNOHANG, 0, 0))
+		return;
 
-	tst_exit();
-}
+	TST_CHECKPOINT_WAKE2(0, MAXKIDS);
 
-static void setup_sigint(void)
-{
-	if (signal(SIGINT, inthandlr) == SIG_ERR)
-		tst_brkm(TFAIL | TERRNO, NULL, "signal SIGINT failed");
-}
+	if (reap_children(0, WNOHANG, fork_kid_pid + (MAXKIDS / 2),
+			  MAXKIDS / 2))
+		return;
 
-static void setup(void)
-{
-	tst_sig(FORK, DEF_HANDLER, cleanup);
+	if (reap_children(-group, WNOHANG, fork_kid_pid, MAXKIDS / 2))
+		return;
 
-	TEST_PAUSE;
+	tst_res(TPASS, "Test PASSED");
 }
 
-static void cleanup(void)
-{
-}
-
-static void inthandlr(void)
-{
-	intintr++;
-}
-
-static void wait_for_parent(void)
-{
-	int testvar;
-
-	while (!intintr)
-		testvar = 0;
-}
-
-static void do_exit(void)
-{
-	wait_for_parent();
-	exit(3);
-}
-
-#ifdef UCLINUX
-static void do_exit_uclinux(void)
-{
-	setup_sigint();
-	do_exit();
-}
-#endif
+static struct tst_test test = {
+	.tid = "waitpid12",
+	.forks_child = 1,
+	.needs_checkpoints = 1,
+	.setup = waitpid_setup,
+	.cleanup = waitpid_cleanup,
+	.test_all = waitpid_test,
+};
-- 
1.7.1


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

* [LTP] [PATCH 7/8] waitpid13: use the new API
  2016-08-10  8:41           ` [LTP] [PATCH 6/8] waitpid12: use the new API Stanislav Kholmanskikh
@ 2016-08-10  8:41             ` Stanislav Kholmanskikh
  2016-08-10  8:41               ` [LTP] [PATCH 8/8] waitpid08: test stopped children Stanislav Kholmanskikh
  2016-08-16 13:24               ` [LTP] [PATCH 7/8] waitpid13: use the new API Cyril Hrubis
  2016-08-16 13:20             ` [LTP] [PATCH 6/8] waitpid12: " Cyril Hrubis
  1 sibling, 2 replies; 31+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-10  8:41 UTC (permalink / raw)
  To: ltp

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/syscalls/waitpid/waitpid13.c      |  398 +++-----------------
 testcases/kernel/syscalls/waitpid/waitpid_common.h |   17 +
 2 files changed, 64 insertions(+), 351 deletions(-)

diff --git a/testcases/kernel/syscalls/waitpid/waitpid13.c b/testcases/kernel/syscalls/waitpid/waitpid13.c
index 5f03592..b6b22b8 100644
--- a/testcases/kernel/syscalls/waitpid/waitpid13.c
+++ b/testcases/kernel/syscalls/waitpid/waitpid13.c
@@ -1,382 +1,78 @@
 /*
+ * Copyright (c) International Business Machines  Corp., 2001
  *
- *   Copyright (c) International Business Machines  Corp., 2001
+ * This program is free software;  you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
  *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY;  without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+ * the GNU General Public License for more details.
  *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
+ * You should have received a copy of the GNU General Public License
+ * along with this program.
  *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ * History
+ *	07/2001 John George
+ *		-Ported
+ *      04/2002 wjhuie sigset cleanups
  */
 
 /*
- * NAME
- *	waitpid13.c
- *
  * DESCRIPTION
- *	Tests to see if pid's returned from fork and waitpid are same
+ *	Tests to see if pids returned from fork and waitpid are same
  *
  * ALGORITHM
  *	Check proper functioning of waitpid with pid = 0 and < -1 with arg
  *	WUNTRACED
- *
- * USAGE:  <for command-line>
- *      waitpid13 [-c n] [-t]
- *      where,  -c n : Run n copies concurrently.
- *              -t   : Turn on syscall timing.
- *
- * History
- *	07/2001 John George
- *		-Ported
- *      04/2002 wjhuie sigset cleanups
- *
- * Restrictions
- *	None
  */
 
-#include <sys/types.h>
-#include <signal.h>
-#include <errno.h>
-#include <sys/wait.h>
-#include "test.h"
-
-#define	MAXKIDS	8
-
-char *TCID = "waitpid13";
-int TST_TOTAL = 1;
-
-volatile int intintr;
-static void setup(void);
-static void cleanup(void);
-static void inthandlr();
-static void wait_for_parent(void);
-static void do_exit(void);
-static void setup_sigint(void);
-#ifdef UCLINUX
-static void do_exit_uclinux(void);
-#endif
-
-static int fail;
+#include "waitpid_common.h"
 
-int main(int ac, char **av)
+static void do_child_1(void)
 {
-	int kid_count, ret_val, status;
-	int i, j, k, found;
-	int group1, group2;
-	int fork_kid_pid[MAXKIDS], wait_kid_pid[MAXKIDS];
-	int pid;
+	pid_t pid, group;
+	int i;
+	int status;
 
-	tst_parse_opts(ac, av, NULL, NULL);
-
-#ifdef UCLINUX
-	maybe_run_child(&do_exit_uclinux, "");
-#endif
-
-	setup();
-
-	tst_count = 0;
-	fail = 0;
-
-	/*
-	 * Need to have test run from child as test driver causes
-	 * test to be a session leader and setpgrp fails.
-	 */
-	pid = FORK_OR_VFORK();
-	if (pid > 0) {
-		waitpid(pid, &status, 0);
-		if (WEXITSTATUS(status) != 0) {
-			tst_resm(TFAIL, "child returned bad status");
-			fail = 1;
-		}
-		if (fail)
-			tst_resm(TFAIL, "%s FAILED", TCID);
-		else
-			tst_resm(TPASS, "%s PASSED", TCID);
-
-		cleanup();
-		tst_exit();
-	} else if (pid < 0) {
-		tst_brkm(TBROK, cleanup, "fork failed");
-	}
-
-	/*
-	 * Set up to catch SIGINT.  The kids will wait till a SIGINT
-	 * has been received before they proceed.
-	 */
-	setup_sigint();
-
-	group1 = getpgrp();
-
-	for (kid_count = 0; kid_count < MAXKIDS; kid_count++) {
-		if (kid_count == (MAXKIDS / 2))
-			group2 = setpgrp();
-
-		intintr = 0;
-		ret_val = FORK_OR_VFORK();
-		if (ret_val == 0) {
-#ifdef UCLINUX
-			if (self_exec(av[0], "") < 0) {
-				tst_resm(TFAIL | TERRNO, "self_exec kid %d "
-					 "failed", kid_count);
-
-			}
-#else
-			do_exit();
-#endif
-		}
-
-		if (ret_val < 0)
-			tst_resm(TFAIL | TERRNO, "forking kid %d failed",
-				 kid_count);
-
-		/* parent */
-		fork_kid_pid[kid_count] = ret_val;
-	}
-
-	/* Check that waitpid with WNOHANG|WUNTRACED returns zero */
-	ret_val = waitpid(0, &status, WNOHANG | WUNTRACED);
-	if (ret_val != 0) {
-		tst_resm(TFAIL, "Waitpid returned wrong value"
-			 "from waitpid(WNOHANG|WUNTRACED)");
-		tst_resm(TFAIL, "Expected 0 got %d", ret_val);
-		fail = 1;
-	}
-#ifdef UCLINUX
-	/* Give the kids a chance to setup SIGINT again, since this is
-	 * cleared by exec().
-	 */
-	sleep(3);
-#endif
+	group = SAFE_GETPGID(0);
 
-	/* Now send all the kids a SIGINT to tell them to proceed */
 	for (i = 0; i < MAXKIDS; i++) {
-		if (kill(fork_kid_pid[i], SIGINT) < 0) {
-			tst_resm(TFAIL | TERRNO, "killing child %d failed", i);
-			fail = 1;
-		}
-	}
-
-	/*
-	 * Wait till all kids have terminated.  Stash away their
-	 * pid's in an array.
-	 */
-	kid_count = 0;
-	errno = 0;
-	while (((ret_val = waitpid(0, &status, WUNTRACED)) != -1) ||
-	       (errno == EINTR)) {
-		if (ret_val == -1)
-			continue;
-
-		if (!WIFEXITED(status)) {
-			if (!WIFSTOPPED(status)) {
-				tst_resm(TFAIL, "Child %d is not "
-					 "stopped", ret_val);
-				fail = 1;
-			} else {
-				if (WSTOPSIG(status) != SIGSTOP) {
-					tst_resm(TFAIL, "Child %d "
-						 "exited with wrong "
-						 "status", ret_val);
-					tst_resm(TFAIL, "Expected "
-						 "SIGSTOP got %d",
-						 WEXITSTATUS(status));
-					fail = 1;
-				}
-			}
-			if (kill(ret_val, SIGCONT) < 0) {
-				tst_resm(TFAIL | TERRNO,
-					 "killing child %d failed", ret_val);
-				fail = 1;
-			}
-		}
-		found = 0;
-		for (j = 0; j < kid_count; j++) {
-			if (ret_val == wait_kid_pid[j]) {
-				found = 1;
-				break;
-			}
-		}
-		if (!found)
-			wait_kid_pid[kid_count++] = ret_val;
-	}
-
-	/*
-	 * Check that for every entry in the fork_kid_pid array,
-	 * there is a matching pid in the wait_kid_pid array.  If
-	 * not, it's an error.
-	 */
-	for (i = 0; i < kid_count; i++) {
-		found = 0;
-		for (j = (MAXKIDS / 2); j < MAXKIDS; j++) {
-			if (fork_kid_pid[j] == wait_kid_pid[i]) {
-				found = 1;
-				break;
-			}
-		}
-		if (!found) {
-			tst_resm(TFAIL, "Did not find a wait_kid_pid "
-				 "for the fork_kid_pid of %d", wait_kid_pid[i]);
-			for (k = 0; k < MAXKIDS; k++)
-				tst_resm(TFAIL, "fork_kid_pid[%d] = "
-					 "%d", k, fork_kid_pid[k]);
-			for (k = 0; k < kid_count; k++)
-				tst_resm(TFAIL, "wait_kid_pid[%d] = "
-					 "%d", k, wait_kid_pid[k]);
-			fail = 1;
-		}
-	}
-
-	if (kid_count != (MAXKIDS / 2)) {
-		tst_resm(TFAIL, "Wrong number of children waited on "
-			 "for pid = 0");
-		tst_resm(TFAIL, "Expected %d got %d", MAXKIDS, kid_count);
-		fail = 1;
-	}
+		if (i == (MAXKIDS / 2))
+			SAFE_SETPGID(0, 0);
 
-	/* Make sure can pickup children in a diff. process group */
+		pid = SAFE_FORK();
+		if (pid == 0)
+			do_exit(1);
 
-	kid_count = 0;
-	errno = 0;
-	while (((ret_val = waitpid(-(group1), &status, WUNTRACED)) !=
-		-1) || (errno == EINTR)) {
-		if (ret_val == -1)
-			continue;
-		if (!WIFEXITED(status)) {
-			if (!WIFSTOPPED(status)) {
-				tst_resm(TFAIL, "Child %d is not "
-					 "stopped", ret_val);
-				fail = 1;
-			} else {
-				if (WSTOPSIG(status) != SIGSTOP) {
-					tst_resm(TFAIL, "Child %d "
-						 "exited with wrong "
-						 "status", ret_val);
-					tst_resm(TFAIL, "Expected "
-						 "SIGSTOP got %d",
-						 WEXITSTATUS(status));
-					fail = 1;
-				}
-			}
-			if (kill(ret_val, SIGCONT) < 0) {
-				tst_resm(TFAIL | TERRNO,
-					 "Killing child %d failed", ret_val);
-				fail = 1;
-			}
-		}
-		found = 0;
-		for (j = 0; j < kid_count; j++) {
-			if (ret_val == wait_kid_pid[j]) {
-				found = 1;
-				break;
-			}
-		}
-		if (!found)
-			wait_kid_pid[kid_count++] = ret_val;
+		fork_kid_pid[i] = pid;
 	}
 
-	/*
-	 * Check that for every entry in the fork_kid_pid array,
-	 * there is a matching pid in the wait_kid_pid array.  If
-	 * not, it's an error.
-	 */
-	for (i = 0; i < kid_count; i++) {
-		found = 0;
-		for (j = 0; j < (MAXKIDS / 2); j++) {
-			if (fork_kid_pid[j] == wait_kid_pid[i]) {
-				found = 1;
-				break;
-			}
-		}
-		if (!found) {
-			tst_resm(TFAIL, "Did not find a wait_kid_pid "
-				 "for the fork_kid_pid of %d", fork_kid_pid[j]);
-			for (k = 0; k < MAXKIDS; k++)
-				tst_resm(TFAIL, "fork_kid_pid[%d] = "
-					 "%d", k, fork_kid_pid[k]);
-			for (k = 0; k < kid_count; k++)
-				tst_resm(TFAIL, "wait_kid_pid[%d] = "
-					 "%d", k, wait_kid_pid[k]);
-			fail = 1;
-		}
-	}
-	if (kid_count != (MAXKIDS / 2)) {
-		tst_resm(TFAIL, "Wrong number of children waited on "
-			 "for pid = 0");
-		tst_resm(TFAIL, "Expected %d got %d", MAXKIDS, kid_count);
-		fail = 1;
-	}
-
-	/*
-	 * Check that waitpid(WUNTRACED) returns -1 when no stopped
-	 * children
-	 */
-	ret_val = waitpid(-1, &status, WUNTRACED);
-	if (ret_val != -1) {
-		tst_resm(TFAIL, "Waitpid returned wrong value.");
-		tst_resm(TFAIL, "Expected -1 got %d", ret_val);
-		fail = 1;
-	}
-
-	if (errno != ECHILD) {
-		tst_resm(TFAIL, "Expected ECHILD from waitpid(WUNTRACED)");
-		fail = 1;
-	}
-
-	if (fail)
-		tst_resm(TFAIL, "Test FAILED");
-	else
-		tst_resm(TPASS, "Test PASSED");
-
-	tst_exit();
-}
-
-static void setup_sigint(void)
-{
-	if (signal(SIGINT, inthandlr) == SIG_ERR)
-		tst_brkm(TFAIL | TERRNO, NULL, "signal SIGINT failed");
-}
+	if (waitpid_ret_test(0, &status, WNOHANG | WUNTRACED, 0, 0))
+		return;
 
-static void setup(void)
-{
-	TEST_PAUSE;
-}
-
-static void cleanup(void)
-{
-}
+	if (waitpid_ret_test(-group, &status, WNOHANG | WUNTRACED, 0, 0))
+		return;
 
-static void inthandlr(void)
-{
-	intintr++;
-}
+	TST_CHECKPOINT_WAKE2(0, MAXKIDS);
 
-static void wait_for_parent(void)
-{
-	int testvar;
+	if (reap_children(0, WUNTRACED, fork_kid_pid + (MAXKIDS / 2),
+			  MAXKIDS / 2))
+		return;
 
-	while (!intintr)
-		testvar = 0;
-}
+	if (reap_children(-group, WUNTRACED, fork_kid_pid, MAXKIDS / 2))
+		return;
 
-static void do_exit(void)
-{
-	wait_for_parent();
-	kill(getpid(), SIGSTOP);
-	exit(3);
+	tst_res(TPASS, "Test PASSED");
 }
 
-#ifdef UCLINUX
-static void do_exit_uclinux(void)
-{
-	setup_sigint();
-	do_exit();
-}
-#endif
+static struct tst_test test = {
+	.tid = "waitpid13",
+	.forks_child = 1,
+	.needs_checkpoints = 1,
+	.setup = waitpid_setup,
+	.cleanup = waitpid_cleanup,
+	.test_all = waitpid_test,
+};
diff --git a/testcases/kernel/syscalls/waitpid/waitpid_common.h b/testcases/kernel/syscalls/waitpid/waitpid_common.h
index a2eb508..b17190f 100644
--- a/testcases/kernel/syscalls/waitpid/waitpid_common.h
+++ b/testcases/kernel/syscalls/waitpid/waitpid_common.h
@@ -131,6 +131,23 @@ static int reap_children(pid_t wp_pid, int wp_opts, pid_t *children, int len)
 			return -1;
 		}
 
+		if (WIFSTOPPED(status)) {
+			if (WSTOPSIG(status) != SIGSTOP) {
+				tst_res(TFAIL,
+					"Pid %d: expected SIGSTOP, got %d",
+					pid, WSTOPSIG(status));
+				return -1;
+			}
+
+			if (kill(pid, SIGCONT) < 0) {
+				tst_res(TFAIL | TERRNO,
+					"kill(%d, SIGCONT) failed", pid);
+				return -1;
+			}
+
+			continue;
+		}
+
 		for (i = 0; i < len; i++) {
 			if (pid == children[i]) {
 				children[i] = 0;
-- 
1.7.1


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

* [LTP] [PATCH 8/8] waitpid08: test stopped children
  2016-08-10  8:41             ` [LTP] [PATCH 7/8] waitpid13: " Stanislav Kholmanskikh
@ 2016-08-10  8:41               ` Stanislav Kholmanskikh
  2016-08-16 13:03                 ` Cyril Hrubis
  2016-08-16 13:24               ` [LTP] [PATCH 7/8] waitpid13: use the new API Cyril Hrubis
  1 sibling, 1 reply; 31+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-10  8:41 UTC (permalink / raw)
  To: ltp

The whole purpose of WUNTRACED is to help with handling of stopped
children. Therefore, let's expand the scope of testing by making
the children stop before we call waitpid() (similarly to waitpid13).

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/syscalls/waitpid/waitpid08.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/testcases/kernel/syscalls/waitpid/waitpid08.c b/testcases/kernel/syscalls/waitpid/waitpid08.c
index e17663f..c743c26 100644
--- a/testcases/kernel/syscalls/waitpid/waitpid08.c
+++ b/testcases/kernel/syscalls/waitpid/waitpid08.c
@@ -43,7 +43,7 @@ static void do_child_1(void)
 
 		pid = SAFE_FORK();
 		if (pid == 0)
-			do_exit(0);
+			do_exit(1);
 
 		fork_kid_pid[i] = pid;
 	}
-- 
1.7.1


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

* [LTP] [PATCH 1/8] waitpid09: use the new API
  2016-08-10  8:40 ` [LTP] [PATCH 1/8] waitpid09: use the new API Stanislav Kholmanskikh
  2016-08-10  8:41   ` [LTP] [PATCH 2/8] waitpid10: " Stanislav Kholmanskikh
@ 2016-08-15 13:55   ` Cyril Hrubis
  2016-08-18  7:40     ` [LTP] [PATCH 1/8 V2] " Stanislav Kholmanskikh
  1 sibling, 1 reply; 31+ messages in thread
From: Cyril Hrubis @ 2016-08-15 13:55 UTC (permalink / raw)
  To: ltp

Hi!
> +static void case1(void)
>  {
> -	intintr++;
> +	pid_t pid, ret;
> +	int status;
> +
> +	pid = SAFE_FORK();
> +	if (pid == 0)
> +		exit(0);
> +
> +	for (;;) {
> +		ret = waitpid(pid, &status, WNOHANG);
> +
> +		if ((ret == -1) && (errno == EINTR))
> +			continue;
> +		if (ret == 0)
> +			continue;
> +
> +		if (ret == pid)
> +			break;
> +
> +		tst_res(TFAIL, "waitpid(WNOHANG) returned %d, expected %d",
> +			ret, pid);
> +		cleanup_pid(pid);
> +		return;
> +	}
> +
> +	if (!WIFEXITED(status)) {
> +		tst_res(TFAIL, "Child exited abnormally");
> +		return;
> +	}
> +
> +	if (WEXITSTATUS(status) != 0) {
> +		tst_res(TFAIL, "Child exited with %d, expected 0",
> +			WEXITSTATUS(status));
> +		return;
> +	}
> +
> +	tst_res(TPASS, "Case 1 PASSED");
>  }

Isn't this one the same as the second part of case0() ?

I guess that we can just do:

	TST_CHECKPOINT_WAKE(0);
	SAFE_WAITPID(pid, NULL, 0);

In the case0() since the rest of the assertions is covered in case1().


Also we may print something little more informative than "Case 1
PASSED". I would do something as:

tst_res(TPASS | TERRNO, "waitpid(-1, ..., WNOHANG)");

But that is very minor.

> +static void waitpid09_test(unsigned int id)
>  {
> -	setup_sigint();
> -	do_exit();
> +	switch (id) {
> +	case 0:
> +		case0();
> +		break;
> +	case 1:
> +		case1();
> +		break;
> +	case 2:
> +		case2();
> +		break;
> +	case 3:
> +		case3();
> +		break;
> +	default:
> +		tst_brk(TBROK, "Unknown %d", id);
> +	}
>  }

This maybe could be a little shorter with an array of fucnctions.

Something as:

static void (*tests[])(void) = {case0, case1, case2, case3};

static void do_test(unsigned int i)
{
	tests[i]();
}


...

static struct test = {
	...
	.tcnt = ARRAY_SIZE(tests);
};




And we may even add a tests pointer to the struct test structure and
teach the test library to use it if set so that we can pass an array of
functions directly.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 2/8] waitpid10: use the new API
  2016-08-10  8:41   ` [LTP] [PATCH 2/8] waitpid10: " Stanislav Kholmanskikh
  2016-08-10  8:41     ` [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test() Stanislav Kholmanskikh
@ 2016-08-15 14:36     ` Cyril Hrubis
  2016-08-18  8:25       ` Stanislav Kholmanskikh
  1 sibling, 1 reply; 31+ messages in thread
From: Cyril Hrubis @ 2016-08-15 14:36 UTC (permalink / raw)
  To: ltp

Hi!
> +static void do_child_1(void)
>  {

Why do we run the test in the do_child_1() here?

As far as I remember the whole point was to start some of the child
processes in different process group and we don't do that in this test,
so we may as well put this function as the test function into the struct
tst_test.


Otherwise this looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test()
  2016-08-10  8:41     ` [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test() Stanislav Kholmanskikh
  2016-08-10  8:41       ` [LTP] [PATCH 4/8] syscalls/waitpid: make reap_children() fail if errno is not ECHILD Stanislav Kholmanskikh
@ 2016-08-15 15:27       ` Cyril Hrubis
  2016-08-18  9:54         ` Stanislav Kholmanskikh
  1 sibling, 1 reply; 31+ messages in thread
From: Cyril Hrubis @ 2016-08-15 15:27 UTC (permalink / raw)
  To: ltp

Hi!
> Several test cases call waitpid() and then verify the returned
> value and errno set. Moved this code into a function.

One thing I do not like about this that this will report the file and
line from the waitpid_common.h in the test output rather than the
respective line where the check has failed.

We could decleare these as a macros that call the function with __FILE__
and __LINE__ and call the tst_res_() with these instead. But I'm not
sure that this is worth the added complexity...

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 4/8] syscalls/waitpid: make reap_children() fail if errno is not ECHILD
  2016-08-10  8:41       ` [LTP] [PATCH 4/8] syscalls/waitpid: make reap_children() fail if errno is not ECHILD Stanislav Kholmanskikh
  2016-08-10  8:41         ` [LTP] [PATCH 5/8] waitpid11: update the description Stanislav Kholmanskikh
@ 2016-08-15 15:49         ` Cyril Hrubis
  1 sibling, 0 replies; 31+ messages in thread
From: Cyril Hrubis @ 2016-08-15 15:49 UTC (permalink / raw)
  To: ltp

Hi!
Looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 5/8] waitpid11: update the description
  2016-08-10  8:41         ` [LTP] [PATCH 5/8] waitpid11: update the description Stanislav Kholmanskikh
  2016-08-10  8:41           ` [LTP] [PATCH 6/8] waitpid12: use the new API Stanislav Kholmanskikh
@ 2016-08-15 15:51           ` Cyril Hrubis
  1 sibling, 0 replies; 31+ messages in thread
From: Cyril Hrubis @ 2016-08-15 15:51 UTC (permalink / raw)
  To: ltp

Hi!
> --- a/testcases/kernel/syscalls/waitpid/waitpid11.c
> +++ b/testcases/kernel/syscalls/waitpid/waitpid11.c
> @@ -25,7 +25,7 @@
>   *	Tests to see if pids returned from fork and waitpid are same
>   *
>   * ALGORITHM
> - * 	Check proper functioning of waitpid with pid = -1 and arg = 0
> + *	Check proper functioning of waitpid with pid = 0 and < -1 with arg 0
>   */

This one is obviously OK.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 8/8] waitpid08: test stopped children
  2016-08-10  8:41               ` [LTP] [PATCH 8/8] waitpid08: test stopped children Stanislav Kholmanskikh
@ 2016-08-16 13:03                 ` Cyril Hrubis
  2016-08-18 10:12                   ` Stanislav Kholmanskikh
  0 siblings, 1 reply; 31+ messages in thread
From: Cyril Hrubis @ 2016-08-16 13:03 UTC (permalink / raw)
  To: ltp

Hi!
> The whole purpose of WUNTRACED is to help with handling of stopped
> children. Therefore, let's expand the scope of testing by making
> the children stop before we call waitpid() (similarly to waitpid13).
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  testcases/kernel/syscalls/waitpid/waitpid08.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/waitpid/waitpid08.c b/testcases/kernel/syscalls/waitpid/waitpid08.c
> index e17663f..c743c26 100644
> --- a/testcases/kernel/syscalls/waitpid/waitpid08.c
> +++ b/testcases/kernel/syscalls/waitpid/waitpid08.c
> @@ -43,7 +43,7 @@ static void do_child_1(void)
>  
>  		pid = SAFE_FORK();
>  		if (pid == 0)
> -			do_exit(0);
> +			do_exit(1);
>  
>  		fork_kid_pid[i] = pid;

Hmm, shouldn't we send SIGCONT and reap the children at the end of the
do_child_1() after this change?

Since if we pass the -i 100 parameter to the test the waitpid_test()
would continue to fork() children that will stay sigstopped in the
background even after the test exits since the fork_kid_pid[] is changed
in each iteration, half of the children has different process group and
the waitpid_cleanup() is called once at the end of the test. Or do I
miss something?

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 6/8] waitpid12: use the new API
  2016-08-10  8:41           ` [LTP] [PATCH 6/8] waitpid12: use the new API Stanislav Kholmanskikh
  2016-08-10  8:41             ` [LTP] [PATCH 7/8] waitpid13: " Stanislav Kholmanskikh
@ 2016-08-16 13:20             ` Cyril Hrubis
  1 sibling, 0 replies; 31+ messages in thread
From: Cyril Hrubis @ 2016-08-16 13:20 UTC (permalink / raw)
  To: ltp

Hi!
Looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 7/8] waitpid13: use the new API
  2016-08-10  8:41             ` [LTP] [PATCH 7/8] waitpid13: " Stanislav Kholmanskikh
  2016-08-10  8:41               ` [LTP] [PATCH 8/8] waitpid08: test stopped children Stanislav Kholmanskikh
@ 2016-08-16 13:24               ` Cyril Hrubis
  1 sibling, 0 replies; 31+ messages in thread
From: Cyril Hrubis @ 2016-08-16 13:24 UTC (permalink / raw)
  To: ltp

Hi!
Looks good as well.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 1/8 V2] waitpid09: use the new API
  2016-08-15 13:55   ` [LTP] [PATCH 1/8] waitpid09: " Cyril Hrubis
@ 2016-08-18  7:40     ` Stanislav Kholmanskikh
  0 siblings, 0 replies; 31+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-18  7:40 UTC (permalink / raw)
  To: ltp

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
Changes since V1:
 * remove code from case0() which is duplicated in case1()
 * rewrite the tst_res(TPASS) messages
 * use function pointers instead of the switch-case structure

 testcases/kernel/syscalls/waitpid/waitpid09.c |  374 ++++++++----------------
 1 files changed, 125 insertions(+), 249 deletions(-)

diff --git a/testcases/kernel/syscalls/waitpid/waitpid09.c b/testcases/kernel/syscalls/waitpid/waitpid09.c
index 1339c82..43beeff 100644
--- a/testcases/kernel/syscalls/waitpid/waitpid09.c
+++ b/testcases/kernel/syscalls/waitpid/waitpid09.c
@@ -1,57 +1,37 @@
 /*
+ * Copyright (c) International Business Machines  Corp., 2001
  *
- *   Copyright (c) International Business Machines  Corp., 2001
+ * This program is free software;  you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
  *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY;  without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+ * the GNU General Public License for more details.
  *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-/*
- * NAME
- *	waitpid09.c
- *
- * DESCRIPTION
- *	Check ability of parent to wait until child returns, and that the
- *	child's process id is returned through the waitpid. Check that
- *	waitpid returns immediately if no child is present.
- *
- * ALGORITHM
- *	case 0:
- *		Parent forks a child and waits. Parent should do nothing
- *		further until child returns. The pid of the forked child
- *		should match the returned value from the waitpid.
- *
- *	case 1:
- *		Parent calls a waitpid with no children waiting. Waitpid
- *		should return a -1 since there are no children to wait for.
- *
- * USAGE:  <for command-line>
- *      waitpid09 [-c n] [-e] [-i n] [-I x] [-P x] [-t]
- *      where,  -c n : Run n copies concurrently.
- *              -e   : Turn on errno logging.
- *              -i n : Execute test n times.
- *              -I x : Execute test for x seconds.
- *              -P x : Pause for x seconds between iterations.
- *              -t   : Turn on syscall timing.
+ * You should have received a copy of the GNU General Public License
+ * along with this program.
  *
  * History
  *	07/2001 John George
  *		-Ported
  *      04/2002 wjhuie sigset cleanups
- *
- * Restrictions
- *	None
+ */
+
+/*
+ * case 0:
+ *        waitpid(pid, WNOHANG) should return 0 if there is a running child
+ * case 1:
+ *        waitpid(pid, WNOHANG) should return the pid of the child if
+ *        the child has exited
+ * case 2:
+ *        waitpid(-1, 0) should return -1 with ECHILD if
+ *        there are no children to wait for.
+ * case 3:
+ *        waitpid(-1, WNOHANG) should return -1 with ECHILD if
+ *        there are no children to wait for.
  */
 
 #define _GNU_SOURCE 1
@@ -60,243 +40,139 @@
 #include <errno.h>
 #include <sys/wait.h>
 #include <stdlib.h>
+#include "tst_test.h"
 
-#include "test.h"
-
-char *TCID = "waitpid09";
-int TST_TOTAL = 1;
-volatile int intintr;
-
-static void setup(void);
-static void cleanup(void);
-static void inthandlr();
-static void do_exit(void);
-static void setup_sigint(void);
-#ifdef UCLINUX
-static void do_exit_uclinux(void);
-#endif
-
-int main(int argc, char **argv)
+static void cleanup_pid(pid_t pid)
 {
-	int lc;
-
-	int fail, pid, status, ret;
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-#ifdef UCLINUX
-	maybe_run_child(&do_exit_uclinux, "");
-#endif
-
-	setup();
-
-	pid = FORK_OR_VFORK();
-	if (pid < 0) {
-		tst_brkm(TFAIL, cleanup, "Fork Failed");
-	} else if (pid == 0) {
-		/*
-		 * Child:
-		 * Set up to catch SIGINT.  The kids will wait till a
-		 * SIGINT has been received before they proceed.
-		 */
-		setup_sigint();
-
-		/* check for looping state if -i option is given */
-		for (lc = 0; TEST_LOOPING(lc); lc++) {
-			/* reset tst_count in case we are looping */
-			tst_count = 0;
-
-			intintr = 0;
+	if (pid > 0) {
+		kill(pid, SIGKILL);
+		waitpid(pid, NULL, 0);
+	}
+}
 
-			fail = 0;
-			pid = FORK_OR_VFORK();
-			if (pid < 0) {
-				tst_brkm(TFAIL, cleanup, "Fork failed.");
-			} else if (pid == 0) {	/* child */
-#ifdef UCLINUX
-				if (self_exec(argv[0], "") < 0) {
-					tst_brkm(TFAIL, cleanup,
-						 "self_exec failed");
-				}
-#else
-				do_exit();
-#endif
-			} else {	/* parent */
+static void case0(void)
+{
+	pid_t pid, ret;
+	int status;
 
-				/*
-				 *Check that waitpid with WNOHANG returns zero
-				 */
-				while (((ret = waitpid(pid, &status, WNOHANG))
-					!= 0) || (errno == EINTR)) {
-					if (ret == -1)
-						continue;
+	pid = SAFE_FORK();
+	if (pid == 0) {
+		TST_CHECKPOINT_WAIT(0);
 
-					tst_resm(TFAIL, "return value for "
-						 "WNOHANG expected 0 got %d",
-						 ret);
-					fail = 1;
-				}
-#ifdef UCLINUX
-				/* Give the kids a chance to setup SIGINT again, since
-				 * this is cleared by exec().
-				 */
-				sleep(3);
-#endif
+		exit(0);
+	}
 
-				/* send SIGINT to child to tell it to proceed */
-				if (kill(pid, SIGINT) < 0) {
-					tst_resm(TFAIL, "Kill of child failed, "
-						 "errno = %d", errno);
-					fail = 1;
-				}
+	for (;;) {
+		ret = waitpid(pid, &status, WNOHANG);
 
-				while (((ret = waitpid(pid, &status, 0)) != -1)
-				       || (errno == EINTR)) {
-					if (ret == -1)
-						continue;
+		if ((ret == -1) && (errno == EINTR))
+			continue;
 
-					if (ret != pid) {
-						tst_resm(TFAIL, "Expected %d "
-							 "got %d as proc id of "
-							 "child", pid, ret);
-						fail = 1;
-					}
+		if (ret == 0)
+			break;
 
-					if (status != 0) {
-						tst_resm(TFAIL, "status value "
-							 "got %d expected 0",
-							 status);
-						fail = 1;
-					}
-				}
-			}
+		tst_res(TFAIL, "waitpid(WNOHANG) returned %d, expected 0",
+			ret);
+		cleanup_pid(pid);
+		return;
+	}
 
-			pid = FORK_OR_VFORK();
-			if (pid < 0) {
-				tst_brkm(TFAIL, cleanup, "Second fork failed.");
-			} else if (pid == 0) {	/* child */
-				exit(0);
-			} else {	/* parent */
-				/* Give the child time to startup and exit */
-				sleep(2);
+	TST_CHECKPOINT_WAKE(0);
+	SAFE_WAITPID(pid, NULL, 0);
 
-				while (((ret = waitpid(pid, &status, WNOHANG))
-					!= -1) || (errno == EINTR)) {
-					if (ret == -1)
-						continue;
+	tst_res(TPASS, "waitpid(pid, WNOHANG) = 0 for a running child");
+}
 
-					if (ret != pid) {
-						tst_resm(TFAIL, "proc id %d "
-							 "and retval %d do not "
-							 "match", pid, ret);
-						fail = 1;
-					}
+static void case1(void)
+{
+	pid_t pid, ret;
+	int status;
 
-					if (status != 0) {
-						tst_resm(TFAIL, "non zero "
-							 "status received %d",
-							 status);
-						fail = 1;
-					}
-				}
-			}
+	pid = SAFE_FORK();
+	if (pid == 0)
+		exit(0);
 
-			if (fail)
-				tst_resm(TFAIL, "case 1 FAILED");
-			else
-				tst_resm(TPASS, "case 1 PASSED");
+	for (;;) {
+		ret = waitpid(pid, &status, WNOHANG);
 
-			fail = 0;
-			ret = waitpid(pid, &status, 0);
+		if ((ret == -1) && (errno == EINTR))
+			continue;
+		if (ret == 0)
+			continue;
 
-			if (ret != -1) {
-				tst_resm(TFAIL, "Expected -1 got %d", ret);
-				fail = 1;
-			}
-			if (errno != ECHILD) {
-				tst_resm(TFAIL, "Expected ECHILD got %d",
-					 errno);
-				fail = 1;
-			}
+		if (ret == pid)
+			break;
 
-			ret = waitpid(pid, &status, WNOHANG);
-			if (ret != -1) {
-				tst_resm(TFAIL, "WNOHANG: Expected -1 got %d",
-					 ret);
-				fail = 1;
-			}
-			if (errno != ECHILD) {
-				tst_resm(TFAIL, "WNOHANG: Expected ECHILD got "
-					 "%d", errno);
-				fail = 1;
-			}
+		tst_res(TFAIL, "waitpid(WNOHANG) returned %d, expected %d",
+			ret, pid);
+		cleanup_pid(pid);
+		return;
+	}
 
-			if (fail)
-				tst_resm(TFAIL, "case 2 FAILED");
-			else
-				tst_resm(TPASS, "case 2 PASSED");
-		}
+	if (!WIFEXITED(status)) {
+		tst_res(TFAIL, "Child exited abnormally");
+		return;
+	}
 
-		cleanup();
-	} else {
-		/* wait for the child to return */
-		waitpid(pid, &status, 0);
-		if (WEXITSTATUS(status) != 0) {
-			tst_brkm(TBROK, cleanup, "child returned bad "
-				 "status");
-		}
+	if (WEXITSTATUS(status) != 0) {
+		tst_res(TFAIL, "Child exited with %d, expected 0",
+			WEXITSTATUS(status));
+		return;
 	}
 
-	tst_exit();
+	tst_res(TPASS, "waitpid(pid, WNOHANG) = pid for an exited child");
 }
 
-/*
- * setup_sigint()
- *	sets up a SIGINT handler
- */
-static void setup_sigint(void)
+static void case2(void)
 {
-	if ((sig_t) signal(SIGINT, inthandlr) == SIG_ERR) {
-		tst_brkm(TFAIL, cleanup, "signal SIGINT failed, errno = %d",
-			 errno);
+	pid_t ret;
+	int status;
+
+	ret = waitpid(-1, &status, 0);
+
+	if (ret != -1) {
+		tst_res(TFAIL, "Expected -1, got %d", ret);
+		return;
+	}
+	if (errno != ECHILD) {
+		tst_res(TFAIL, "Expected %s, got %s",
+			tst_strerrno(ECHILD), tst_strerrno(errno));
+		return;
 	}
-}
 
-static void setup(void)
-{
-	TEST_PAUSE;
+	tst_res(TPASS, "waitpid(-1, 0) = -1 with ECHILD if no children");
 }
 
-static void cleanup(void)
+static void case3(void)
 {
-}
+	pid_t ret;
+	int status;
 
-static void inthandlr(void)
-{
-	intintr++;
-}
+	ret = waitpid(-1, &status, WNOHANG);
+	if (ret != -1) {
+		tst_res(TFAIL, "WNOHANG: Expected -1, got %d", ret);
+		return;
+	}
+	if (errno != ECHILD) {
+		tst_res(TFAIL, "WNOHANG: Expected %s, got %s",
+			tst_strerrno(ECHILD), tst_strerrno(errno));
+		return;
+	}
 
-static void wait_for_parent(void)
-{
-	int testvar;
-	while (!intintr)
-		testvar = 0;
+	tst_res(TPASS, "waitpid(-1, WNOHANG) = -1 with ECHILD if no children");
 }
 
-static void do_exit(void)
-{
-	wait_for_parent();
-	exit(0);
-}
+static void (*tests[])(void) = { case0, case1, case2, case3 };
 
-#ifdef UCLINUX
-/*
- * do_exit_uclinux()
- *	Sets up SIGINT handler again, then calls do_exit
- */
-static void do_exit_uclinux(void)
+static void waitpid09_test(unsigned int id)
 {
-	setup_sigint();
-	do_exit();
+	tests[id]();
 }
-#endif
+
+static struct tst_test test = {
+	.tid = "waitpid09",
+	.forks_child = 1,
+	.needs_checkpoints = 1,
+	.test = waitpid09_test,
+	.tcnt = ARRAY_SIZE(tests),
+};
-- 
1.7.1


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

* [LTP] [PATCH 2/8] waitpid10: use the new API
  2016-08-15 14:36     ` [LTP] [PATCH 2/8] waitpid10: use the new API Cyril Hrubis
@ 2016-08-18  8:25       ` Stanislav Kholmanskikh
  2016-08-18  8:33         ` Stanislav Kholmanskikh
  0 siblings, 1 reply; 31+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-18  8:25 UTC (permalink / raw)
  To: ltp



On 08/15/2016 05:36 PM, Cyril Hrubis wrote:
> Hi!
>> +static void do_child_1(void)
>>  {
> 
> Why do we run the test in the do_child_1() here?
> 
> As far as I remember the whole point was to start some of the child
> processes in different process group and we don't do that in this test,
> so we may as well put this function as the test function into the struct
> tst_test.

waitpid_test() just forks a child - do_child_1() which then does the
actual testing. This scheme is needed in other test cases, since they
operate with process groups.

Yes, here it's redundant, since this do_child_1() doesn't touch process
groups. I decided to use this scheme here just for similarity with other
test cases. In my opinion, it brings no harm or improvements.


> 
> 
> Otherwise this looks good.
> 

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

* [LTP] [PATCH 2/8] waitpid10: use the new API
  2016-08-18  8:25       ` Stanislav Kholmanskikh
@ 2016-08-18  8:33         ` Stanislav Kholmanskikh
  2016-08-18  9:19           ` Cyril Hrubis
  0 siblings, 1 reply; 31+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-18  8:33 UTC (permalink / raw)
  To: ltp



On 08/18/2016 11:25 AM, Stanislav Kholmanskikh wrote:
> 
> 
> On 08/15/2016 05:36 PM, Cyril Hrubis wrote:
>> Hi!
>>> +static void do_child_1(void)
>>>  {
>>
>> Why do we run the test in the do_child_1() here?
>>
>> As far as I remember the whole point was to start some of the child
>> processes in different process group and we don't do that in this test,
>> so we may as well put this function as the test function into the struct
>> tst_test.
> 
> waitpid_test() just forks a child - do_child_1() which then does the
> actual testing. This scheme is needed in other test cases, since they
> operate with process groups.
> 
> Yes, here it's redundant, since this do_child_1() doesn't touch process
> groups. I decided to use this scheme here just for similarity with other
> test cases. In my opinion, it brings no harm or improvements.

I can add this to the commit message.


> 
> 
>>
>>
>> Otherwise this looks good.
>>
> 

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

* [LTP] [PATCH 2/8] waitpid10: use the new API
  2016-08-18  8:33         ` Stanislav Kholmanskikh
@ 2016-08-18  9:19           ` Cyril Hrubis
  0 siblings, 0 replies; 31+ messages in thread
From: Cyril Hrubis @ 2016-08-18  9:19 UTC (permalink / raw)
  To: ltp

Hi!
> > waitpid_test() just forks a child - do_child_1() which then does the
> > actual testing. This scheme is needed in other test cases, since they
> > operate with process groups.
> > 
> > Yes, here it's redundant, since this do_child_1() doesn't touch process
> > groups. I decided to use this scheme here just for similarity with other
> > test cases. In my opinion, it brings no harm or improvements.
> 
> I can add this to the commit message.

Ok

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test()
  2016-08-15 15:27       ` [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test() Cyril Hrubis
@ 2016-08-18  9:54         ` Stanislav Kholmanskikh
  2016-08-18 10:42           ` Cyril Hrubis
  0 siblings, 1 reply; 31+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-18  9:54 UTC (permalink / raw)
  To: ltp

Hi!

On 08/15/2016 06:27 PM, Cyril Hrubis wrote:
> Hi!
>> Several test cases call waitpid() and then verify the returned
>> value and errno set. Moved this code into a function.
> 
> One thing I do not like about this that this will report the file and
> line from the waitpid_common.h in the test output rather than the
> respective line where the check has failed.
> 
> We could decleare these as a macros that call the function with __FILE__
> and __LINE__ and call the tst_res_() with these instead. But I'm not
> sure that this is worth the added complexity...
> 

So you mean something like the attached function. Right?

With this code a failure will be presented as:

[stas@kholmanskikh waitpid]$ ./waitpid07
tst_test.c:756: INFO: Timeout per run is 0h 05m 00s
waitpid07.c:51: FAIL: waitpid() returned 0, expected 666

whereas with the original code:

[stas@kholmanskikh waitpid]$ ./waitpid07
tst_test.c:756: INFO: Timeout per run is 0h 05m 00s
waitpid_common.h:97: FAIL: waitpid() returned 0, expected 666

I.e. in the former case a user will be given the function which failed
and will need to go to its code to find the corresponding tst_res(TFAIL)
call, whereas with the original code he/she will be given the
tst_res(TFAIL) call, but will need to manually find a corresponding
function call in the test case sources. Yes, the former case is more
user friendly, but, to be honest, I don't think it's worth the added
complexity.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: waitpid_ret_test.c
Type: text/x-csrc
Size: 752 bytes
Desc: not available
URL: <http://lists.linux.it/pipermail/ltp/attachments/20160818/02c61051/attachment.c>

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

* [LTP] [PATCH 8/8] waitpid08: test stopped children
  2016-08-16 13:03                 ` Cyril Hrubis
@ 2016-08-18 10:12                   ` Stanislav Kholmanskikh
  2016-08-18 10:48                     ` Cyril Hrubis
  0 siblings, 1 reply; 31+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-18 10:12 UTC (permalink / raw)
  To: ltp

Hi!

On 08/16/2016 04:03 PM, Cyril Hrubis wrote:
> Hi!
>> The whole purpose of WUNTRACED is to help with handling of stopped
>> children. Therefore, let's expand the scope of testing by making
>> the children stop before we call waitpid() (similarly to waitpid13).
>>
>> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
>> ---
>>  testcases/kernel/syscalls/waitpid/waitpid08.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/waitpid/waitpid08.c b/testcases/kernel/syscalls/waitpid/waitpid08.c
>> index e17663f..c743c26 100644
>> --- a/testcases/kernel/syscalls/waitpid/waitpid08.c
>> +++ b/testcases/kernel/syscalls/waitpid/waitpid08.c
>> @@ -43,7 +43,7 @@ static void do_child_1(void)
>>  
>>  		pid = SAFE_FORK();
>>  		if (pid == 0)
>> -			do_exit(0);
>> +			do_exit(1);
>>  
>>  		fork_kid_pid[i] = pid;
> 
> Hmm, shouldn't we send SIGCONT and reap the children at the end of the
> do_child_1() after this change?
> 
> Since if we pass the -i 100 parameter to the test the waitpid_test()
> would continue to fork() children that will stay sigstopped in the
> background even after the test exits since the fork_kid_pid[] is changed
> in each iteration, half of the children has different process group and
> the waitpid_cleanup() is called once at the end of the test. Or do I
> miss something?
> 

No-no, this should not happen, since reap_children() handles stopped
children this way:

for (;;) {
  pid = waitpid();

  if (pid is a pid of a stopped task) {
      kill(pid, SIGCONT);
      continue;
  }

  <...>
}

i.e. it makes all the stopped children continue.

So there should not be any children in 'pgroup' after
reap_children(pgroup) has finished.

This SIGSTOP-related code is added by the previous patch in the series
(waitpid13).

Thanks.



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

* [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test()
  2016-08-18  9:54         ` Stanislav Kholmanskikh
@ 2016-08-18 10:42           ` Cyril Hrubis
  2016-08-18 15:15             ` Stanislav Kholmanskikh
  0 siblings, 1 reply; 31+ messages in thread
From: Cyril Hrubis @ 2016-08-18 10:42 UTC (permalink / raw)
  To: ltp

Hi!
> So you mean something like the attached function. Right?

Yes.

> With this code a failure will be presented as:
> 
> [stas@kholmanskikh waitpid]$ ./waitpid07
> tst_test.c:756: INFO: Timeout per run is 0h 05m 00s
> waitpid07.c:51: FAIL: waitpid() returned 0, expected 666
> 
> whereas with the original code:
> 
> [stas@kholmanskikh waitpid]$ ./waitpid07
> tst_test.c:756: INFO: Timeout per run is 0h 05m 00s
> waitpid_common.h:97: FAIL: waitpid() returned 0, expected 666
> 
> I.e. in the former case a user will be given the function which failed
> and will need to go to its code to find the corresponding tst_res(TFAIL)
> call, whereas with the original code he/she will be given the
> tst_res(TFAIL) call, but will need to manually find a corresponding
> function call in the test case sources. Yes, the former case is more
> user friendly, but, to be honest, I don't think it's worth the added
> complexity.

The whole motivation for printing the file and line in the
tst_res()/tst_brk() was to make it easier to analyse failures from test
logs. I.e. somebody posts test failure logs on the ML and you can see
what failed and where just by looking at the logs.

Sure you can add a few more test prints, recompile and run the test and
see what went wrong. But once you have to ask somebody at the other end
to do that and run it on a specific hardware or wait for other tests to
finish on a shared machine just to rerun the test, things get more
complicated.

So I would really want to keep the file and line tied closely to the
place in the source where the failure occured.

Here it could be done either by:

* Passing the file and line as in the snippet you send in this email
  - here we pay a price by making the code more complex

* Implementing the whole check as a macro
  - ugly but does the job

* Keeping the checks in the source code
  - we repeat the same pattern of code over and over there

None of these is really good solution to the problem unfortunately.



There may be a better solution, and I'm thinking of that one for quite
some time. We may be as well able to generate the tests from templates
which is something I would like to explore in the long term. But that
approach has another set of problems on it's own.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 8/8] waitpid08: test stopped children
  2016-08-18 10:12                   ` Stanislav Kholmanskikh
@ 2016-08-18 10:48                     ` Cyril Hrubis
  2016-08-18 11:37                       ` Stanislav Kholmanskikh
  0 siblings, 1 reply; 31+ messages in thread
From: Cyril Hrubis @ 2016-08-18 10:48 UTC (permalink / raw)
  To: ltp

Hi!
> No-no, this should not happen, since reap_children() handles stopped
> children this way:
> 
> for (;;) {
>   pid = waitpid();
> 
>   if (pid is a pid of a stopped task) {
>       kill(pid, SIGCONT);
>       continue;
>   }
> 
>   <...>
> }
> 
> i.e. it makes all the stopped children continue.
> 
> So there should not be any children in 'pgroup' after
> reap_children(pgroup) has finished.
> 
> This SIGSTOP-related code is added by the previous patch in the series
> (waitpid13).

Ah, missed that, sorry.

It probably should have been in a separate patch.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 8/8] waitpid08: test stopped children
  2016-08-18 10:48                     ` Cyril Hrubis
@ 2016-08-18 11:37                       ` Stanislav Kholmanskikh
  0 siblings, 0 replies; 31+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-18 11:37 UTC (permalink / raw)
  To: ltp



On 08/18/2016 01:48 PM, Cyril Hrubis wrote:
> Hi!
>> No-no, this should not happen, since reap_children() handles stopped
>> children this way:
>>
>> for (;;) {
>>   pid = waitpid();
>>
>>   if (pid is a pid of a stopped task) {
>>       kill(pid, SIGCONT);
>>       continue;
>>   }
>>
>>   <...>
>> }
>>
>> i.e. it makes all the stopped children continue.
>>
>> So there should not be any children in 'pgroup' after
>> reap_children(pgroup) has finished.
>>
>> This SIGSTOP-related code is added by the previous patch in the series
>> (waitpid13).
> 
> Ah, missed that, sorry.
> 
> It probably should have been in a separate patch.

Ok. I'll stplit the waitpid13 patch into two:
 * one is for the SIGSTOP changes in watipid_common.h
 * the other is for the actual update to waitpid13.c
> 

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

* [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test()
  2016-08-18 10:42           ` Cyril Hrubis
@ 2016-08-18 15:15             ` Stanislav Kholmanskikh
  2016-08-18 15:43               ` Cyril Hrubis
  2016-08-18 15:54               ` Cyril Hrubis
  0 siblings, 2 replies; 31+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-18 15:15 UTC (permalink / raw)
  To: ltp



On 08/18/2016 01:42 PM, Cyril Hrubis wrote:
> Hi!
>> So you mean something like the attached function. Right?
> 
> Yes.
> 
>> With this code a failure will be presented as:
>>
>> [stas@kholmanskikh waitpid]$ ./waitpid07
>> tst_test.c:756: INFO: Timeout per run is 0h 05m 00s
>> waitpid07.c:51: FAIL: waitpid() returned 0, expected 666
>>
>> whereas with the original code:
>>
>> [stas@kholmanskikh waitpid]$ ./waitpid07
>> tst_test.c:756: INFO: Timeout per run is 0h 05m 00s
>> waitpid_common.h:97: FAIL: waitpid() returned 0, expected 666
>>
>> I.e. in the former case a user will be given the function which failed
>> and will need to go to its code to find the corresponding tst_res(TFAIL)
>> call, whereas with the original code he/she will be given the
>> tst_res(TFAIL) call, but will need to manually find a corresponding
>> function call in the test case sources. Yes, the former case is more
>> user friendly, but, to be honest, I don't think it's worth the added
>> complexity.
> 
> The whole motivation for printing the file and line in the
> tst_res()/tst_brk() was to make it easier to analyse failures from test
> logs. I.e. somebody posts test failure logs on the ML and you can see
> what failed and where just by looking at the logs.
> 
> Sure you can add a few more test prints, recompile and run the test and
> see what went wrong. But once you have to ask somebody at the other end
> to do that and run it on a specific hardware or wait for other tests to
> finish on a shared machine just to rerun the test, things get more
> complicated.
> 
> So I would really want to keep the file and line tied closely to the
> place in the source where the failure occured.
> 
> Here it could be done either by:
> 
> * Passing the file and line as in the snippet you send in this email
>   - here we pay a price by making the code more complex
> 
> * Implementing the whole check as a macro
>   - ugly but does the job

Like this (sorry for formatting):

#define WAITPID_RET_TEST(wp_pid, wp_status, wp_opts, wp_ret, wp_errno)  \
        do {                                                            \
                if (waitpid_ret_test(wp_pid, wp_status,                 \
                                     wp_opts, wp_ret, wp_errno)) {      \
                        tst_res_(__FILE__, __LINE__, TFAIL,             \
                                 "waitpid_ret_test() failed");          \
                        return;                                         \
                }                                                       \
        } while (0)

?

This will produce:

[stas@kholmanskikh waitpid]$ ./waitpid07
tst_test.c:756: INFO: Timeout per run is 0h 05m 00s
waitpid_common.h:97: FAIL: waitpid() returned 0, expected 666
waitpid07.c:51: FAIL: waitpid_ret_test() failed

Summary:
passed   0
failed   2
skipped  0
warnings 0


A similar operation would be required for reap_children().


> 
> * Keeping the checks in the source code
>   - we repeat the same pattern of code over and over there
> 
> None of these is really good solution to the problem unfortunately.
> 
> 
> 
> There may be a better solution, and I'm thinking of that one for quite
> some time. We may be as well able to generate the tests from templates
> which is something I would like to explore in the long term. But that
> approach has another set of problems on it's own.
> 

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

* [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test()
  2016-08-18 15:15             ` Stanislav Kholmanskikh
@ 2016-08-18 15:43               ` Cyril Hrubis
  2016-08-18 15:54               ` Cyril Hrubis
  1 sibling, 0 replies; 31+ messages in thread
From: Cyril Hrubis @ 2016-08-18 15:43 UTC (permalink / raw)
  To: ltp

Hi!
> #define WAITPID_RET_TEST(wp_pid, wp_status, wp_opts, wp_ret, wp_errno)  \
>         do {                                                            \
>                 if (waitpid_ret_test(wp_pid, wp_status,                 \
>                                      wp_opts, wp_ret, wp_errno)) {      \
>                         tst_res_(__FILE__, __LINE__, TFAIL,             \
>                                  "waitpid_ret_test() failed");          \
>                         return;                                         \
>                 }                                                       \
>         } while (0)

You can use just tst_res() there. (It will force the C processor to make
one more pass since the tes_res() would need to be expanded to
tst_res_()...) and the __FILE__ and __LINE__ should be expanded last and
should point to the location of code the macro has been used.

We can put the whole waitpid check into the macro, something as:

#define WAITPID_RET_TEST(waitpid_fn, exp_ret, exp_errno) \
	do {
		pid_t ret = waitpid_fn;
		if (ret != exp_ret) {
			tst_res(TFAIL,
			        #waitpid_fn " returned %i, expected %i",
				ret, exp_ret);
			return;
		}

		if (ret == -1 && errno != exp_errno) {
			tst_res(TFAIL | TERRNO,
			        #waitpid_fn "expected %s",
				tst_strerrno(exp_errno));
			return;
		}
	} while (0);

And call it as:

WAITPID_RET_TEST(waitpid(-1, &status, WNOHANG | WUNTRACED), 0, 0);

> A similar operation would be required for reap_children().

Hmm, that one would be overly complicated and ugly as a macro.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test()
  2016-08-18 15:15             ` Stanislav Kholmanskikh
  2016-08-18 15:43               ` Cyril Hrubis
@ 2016-08-18 15:54               ` Cyril Hrubis
  2016-08-19  9:47                 ` Stanislav Kholmanskikh
  1 sibling, 1 reply; 31+ messages in thread
From: Cyril Hrubis @ 2016-08-18 15:54 UTC (permalink / raw)
  To: ltp

Hi!
> #define WAITPID_RET_TEST(wp_pid, wp_status, wp_opts, wp_ret, wp_errno)  \
>         do {                                                            \
>                 if (waitpid_ret_test(wp_pid, wp_status,                 \
>                                      wp_opts, wp_ret, wp_errno)) {      \
>                         tst_res_(__FILE__, __LINE__, TFAIL,             \
>                                  "waitpid_ret_test() failed");          \
>                         return;                                         \
>                 }                                                       \
>         } while (0)
> 
> ?
> 
> This will produce:
> 
> [stas@kholmanskikh waitpid]$ ./waitpid07
> tst_test.c:756: INFO: Timeout per run is 0h 05m 00s
> waitpid_common.h:97: FAIL: waitpid() returned 0, expected 666
> waitpid07.c:51: FAIL: waitpid_ret_test() failed

I guess that we can go this way and turn the last TFAIL into the TINFO.

Well, we may also do something as:

#define TST_TRACE(expr) \
	({int ret = expr; ret != 0 ? tst_res(TINFO, #expr " failed"), ret : ret;})


Then call it as:

	if (TST_TRACE(waitpid_ret_test(...)))
		return;

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test()
  2016-08-18 15:54               ` Cyril Hrubis
@ 2016-08-19  9:47                 ` Stanislav Kholmanskikh
  2016-08-22 17:38                   ` Cyril Hrubis
  0 siblings, 1 reply; 31+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-19  9:47 UTC (permalink / raw)
  To: ltp



On 08/18/2016 06:54 PM, Cyril Hrubis wrote:
> Hi!
>> #define WAITPID_RET_TEST(wp_pid, wp_status, wp_opts, wp_ret, wp_errno)  \
>>         do {                                                            \
>>                 if (waitpid_ret_test(wp_pid, wp_status,                 \
>>                                      wp_opts, wp_ret, wp_errno)) {      \
>>                         tst_res_(__FILE__, __LINE__, TFAIL,             \
>>                                  "waitpid_ret_test() failed");          \
>>                         return;                                         \
>>                 }                                                       \
>>         } while (0)
>>
>> ?
>>
>> This will produce:
>>
>> [stas@kholmanskikh waitpid]$ ./waitpid07
>> tst_test.c:756: INFO: Timeout per run is 0h 05m 00s
>> waitpid_common.h:97: FAIL: waitpid() returned 0, expected 666
>> waitpid07.c:51: FAIL: waitpid_ret_test() failed
> 
> I guess that we can go this way and turn the last TFAIL into the TINFO.
> 
> Well, we may also do something as:
> 
> #define TST_TRACE(expr) \
> 	({int ret = expr; ret != 0 ? tst_res(TINFO, #expr " failed"), ret : ret;})

I like this idea with '#expr'.

Do you want me to put this in tst_test.h, or is it acceptable if I keep
this in waitpid_common.h? If it's acceptable, I'd rename it to
WAITPID_TEST and used the 'return' statement"

#define WAITPID_TEST(expr) \
   do { \
      if (expr) { \
          tst_rest(TINFO, #expr " failed"); \
          return; \
      } \
   } while (0);

and call it

WAITPID_TEST(waitpid_ret_test());
WAITPID_TEST(reap_children());

> 
> 
> Then call it as:
> 
> 	if (TST_TRACE(waitpid_ret_test(...)))
> 		return;
> 

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

* [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test()
  2016-08-19  9:47                 ` Stanislav Kholmanskikh
@ 2016-08-22 17:38                   ` Cyril Hrubis
  0 siblings, 0 replies; 31+ messages in thread
From: Cyril Hrubis @ 2016-08-22 17:38 UTC (permalink / raw)
  To: ltp

Hi!
> > #define TST_TRACE(expr) \
> > 	({int ret = expr; ret != 0 ? tst_res(TINFO, #expr " failed"), ret : ret;})
> 
> I like this idea with '#expr'.
> 
> Do you want me to put this in tst_test.h, or is it acceptable if I keep
> this in waitpid_common.h? If it's acceptable, I'd rename it to
> WAITPID_TEST and used the 'return' statement"

I guess that this is useful enough to be included in the test library.

I mildy oppose to the return in the macro, since that hides the actuall
code flow. Maybe we can make it clearer by including RET in the macro
name. I couldn't come up with something short and descriptive though,
maybe something as TRACE_RET_ON_FAIL(waitpid_ret_test(...)) or
TRET_ON_FAIL(...).

-- 
Cyril Hrubis
chrubis@suse.cz

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

end of thread, other threads:[~2016-08-22 17:38 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-10  8:40 [LTP] waitpid: new API (part 2) Stanislav Kholmanskikh
2016-08-10  8:40 ` [LTP] [PATCH 1/8] waitpid09: use the new API Stanislav Kholmanskikh
2016-08-10  8:41   ` [LTP] [PATCH 2/8] waitpid10: " Stanislav Kholmanskikh
2016-08-10  8:41     ` [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test() Stanislav Kholmanskikh
2016-08-10  8:41       ` [LTP] [PATCH 4/8] syscalls/waitpid: make reap_children() fail if errno is not ECHILD Stanislav Kholmanskikh
2016-08-10  8:41         ` [LTP] [PATCH 5/8] waitpid11: update the description Stanislav Kholmanskikh
2016-08-10  8:41           ` [LTP] [PATCH 6/8] waitpid12: use the new API Stanislav Kholmanskikh
2016-08-10  8:41             ` [LTP] [PATCH 7/8] waitpid13: " Stanislav Kholmanskikh
2016-08-10  8:41               ` [LTP] [PATCH 8/8] waitpid08: test stopped children Stanislav Kholmanskikh
2016-08-16 13:03                 ` Cyril Hrubis
2016-08-18 10:12                   ` Stanislav Kholmanskikh
2016-08-18 10:48                     ` Cyril Hrubis
2016-08-18 11:37                       ` Stanislav Kholmanskikh
2016-08-16 13:24               ` [LTP] [PATCH 7/8] waitpid13: use the new API Cyril Hrubis
2016-08-16 13:20             ` [LTP] [PATCH 6/8] waitpid12: " Cyril Hrubis
2016-08-15 15:51           ` [LTP] [PATCH 5/8] waitpid11: update the description Cyril Hrubis
2016-08-15 15:49         ` [LTP] [PATCH 4/8] syscalls/waitpid: make reap_children() fail if errno is not ECHILD Cyril Hrubis
2016-08-15 15:27       ` [LTP] [PATCH 3/8] syscalls/waitpid: implement waitpid_ret_test() Cyril Hrubis
2016-08-18  9:54         ` Stanislav Kholmanskikh
2016-08-18 10:42           ` Cyril Hrubis
2016-08-18 15:15             ` Stanislav Kholmanskikh
2016-08-18 15:43               ` Cyril Hrubis
2016-08-18 15:54               ` Cyril Hrubis
2016-08-19  9:47                 ` Stanislav Kholmanskikh
2016-08-22 17:38                   ` Cyril Hrubis
2016-08-15 14:36     ` [LTP] [PATCH 2/8] waitpid10: use the new API Cyril Hrubis
2016-08-18  8:25       ` Stanislav Kholmanskikh
2016-08-18  8:33         ` Stanislav Kholmanskikh
2016-08-18  9:19           ` Cyril Hrubis
2016-08-15 13:55   ` [LTP] [PATCH 1/8] waitpid09: " Cyril Hrubis
2016-08-18  7:40     ` [LTP] [PATCH 1/8 V2] " Stanislav Kholmanskikh

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.