All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH 1/4] syscalls/kill01: Remove it
@ 2020-08-20 10:26 Feiyu Zhu
  2020-08-20 10:26 ` [LTP] [PATCH 2/4] syscalls/kill03, 04: Cleanup && Convert to new library Feiyu Zhu
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Feiyu Zhu @ 2020-08-20 10:26 UTC (permalink / raw)
  To: ltp

kill11 has covered this test, so remove it.

Reviewed-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
Signed-off-by: Feiyu Zhu <zhufy.jy@cn.fujitsu.com>
---
 runtest/syscalls                          |   1 -
 testcases/kernel/syscalls/kill/.gitignore |   1 -
 testcases/kernel/syscalls/kill/kill01.c   | 161 ------------------------------
 3 files changed, 163 deletions(-)
 delete mode 100644 testcases/kernel/syscalls/kill/kill01.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 860c5c3..5e2bdb1 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -614,7 +614,6 @@ kcmp01 kcmp01
 kcmp02 kcmp02
 kcmp03 kcmp03
 
-kill01 kill01
 kill02 kill02
 kill03 kill03
 kill04 kill04
diff --git a/testcases/kernel/syscalls/kill/.gitignore b/testcases/kernel/syscalls/kill/.gitignore
index 490a1e2..b62662f 100644
--- a/testcases/kernel/syscalls/kill/.gitignore
+++ b/testcases/kernel/syscalls/kill/.gitignore
@@ -1,4 +1,3 @@
-/kill01
 /kill02
 /kill03
 /kill04
diff --git a/testcases/kernel/syscalls/kill/kill01.c b/testcases/kernel/syscalls/kill/kill01.c
deleted file mode 100644
index 0aa77b9..0000000
--- a/testcases/kernel/syscalls/kill/kill01.c
+++ /dev/null
@@ -1,161 +0,0 @@
-/*
- *
- *   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 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
- *	kill01.c
- *
- * DESCRIPTION
- *	Test case to check the basic functionality of kill().
- *
- * ALGORITHM
- *	call setup
- *	loop if the -i option was given
- *	fork a child
- *	execute the kill system call
- *	check the return value
- *	if return value is -1
- *		issue a FAIL message, break remaining tests and cleanup
- *	if we are doing functional testing
- *		if the process was terminated with the expected signal.
- *			issue a PASS message
- *		otherwise
- *			issue a FAIL message
- *	call cleanup
- *
- * USAGE
- *  kill01 [-c n] [-f] [-i n] [-I x] [-P x] [-t]
- *     where,  -c n : Run n copies concurrently.
- *             -f   : Turn off functionality Testing.
- *             -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.
- *
- * HISTORY
- *	07/2001 Ported by Wayne Boyer
- *
- * RESTRICTIONS
- *	This test should be ran as a non-root user.
- */
-
-#include "test.h"
-
-#include <signal.h>
-#include <errno.h>
-#include <sys/wait.h>
-
-void cleanup(void);
-void setup(void);
-void do_child(void);
-
-char *TCID = "kill01";
-int TST_TOTAL = 1;
-
-#define TEST_SIG SIGKILL
-
-int main(int ac, char **av)
-{
-	int lc;
-	pid_t pid;
-	int exno, status, nsig;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-#ifdef UCLINUX
-	maybe_run_child(&do_child, "");
-#endif
-
-	setup();		/* global setup */
-
-	/* The following loop checks looping state if -i option given */
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		/* reset tst_count in case we are looping */
-		tst_count = 0;
-		status = 1;
-		exno = 1;
-		pid = FORK_OR_VFORK();
-		if (pid < 0) {
-			tst_brkm(TBROK, cleanup, "Fork of child failed");
-		} else if (pid == 0) {
-#ifdef UCLINUX
-			if (self_exec(av[0], "") < 0) {
-				tst_brkm(TBROK, cleanup,
-					 "self_exec of child failed");
-			}
-#else
-			do_child();
-#endif
-		} else {
-			TEST(kill(pid, TEST_SIG));
-			waitpid(pid, &status, 0);
-		}
-
-		if (TEST_RETURN == -1) {
-			tst_brkm(TFAIL, cleanup, "%s failed - errno = %d : %s",
-				 TCID, TEST_ERRNO, strerror(TEST_ERRNO));
-		}
-
-		/*
-		 * Check to see if the process was terminated with the
-		 * expected signal.
-		 */
-		nsig = WTERMSIG(status);
-		if (nsig == TEST_SIG) {
-			tst_resm(TPASS, "received expected signal %d",
-				 nsig);
-		} else {
-			tst_resm(TFAIL,
-				 "expected signal %d received %d",
-				 TEST_SIG, nsig);
-		}
-	}
-
-	cleanup();
-	tst_exit();
-}
-
-/*
- * do_child()
- */
-void do_child(void)
-{
-	int exno = 1;
-
-	pause();
-	exit(exno);
-}
-
-/*
- * setup() - performs all ONE TIME setup for this test
- */
-void setup(void)
-{
-
-	TEST_PAUSE;
-}
-
-/*
- * cleanup() - performs all the ONE TIME cleanup for this test@completion
- * or premature exit.
- */
-void cleanup(void)
-{
-
-}
-- 
1.8.3.1




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

* [LTP] [PATCH 2/4] syscalls/kill03, 04: Cleanup && Convert to new library
  2020-08-20 10:26 [LTP] [PATCH 1/4] syscalls/kill01: Remove it Feiyu Zhu
@ 2020-08-20 10:26 ` Feiyu Zhu
  2020-08-20 12:18   ` Li Wang
  2020-08-20 10:26 ` [LTP] [PATCH 3/4] syscalls/kill05:Cleanup " Feiyu Zhu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Feiyu Zhu @ 2020-08-20 10:26 UTC (permalink / raw)
  To: ltp

1) Remove uclinux code
2) Take use of safe macro
3) Merge kill04 into kill03
4) Add error test that pid is INT_MIN

Reviewed-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
Signed-off-by: Feiyu Zhu <zhufy.jy@cn.fujitsu.com>
---
 runtest/syscalls                          |   1 -
 testcases/kernel/syscalls/kill/.gitignore |   1 -
 testcases/kernel/syscalls/kill/kill03.c   | 203 +++++++++---------------------
 testcases/kernel/syscalls/kill/kill04.c   | 133 --------------------
 4 files changed, 59 insertions(+), 279 deletions(-)
 delete mode 100644 testcases/kernel/syscalls/kill/kill04.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 5e2bdb1..a6ab75b 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -616,7 +616,6 @@ kcmp03 kcmp03
 
 kill02 kill02
 kill03 kill03
-kill04 kill04
 kill05 kill05
 kill06 kill06
 kill07 kill07
diff --git a/testcases/kernel/syscalls/kill/.gitignore b/testcases/kernel/syscalls/kill/.gitignore
index b62662f..75fdaa5 100644
--- a/testcases/kernel/syscalls/kill/.gitignore
+++ b/testcases/kernel/syscalls/kill/.gitignore
@@ -1,6 +1,5 @@
 /kill02
 /kill03
-/kill04
 /kill05
 /kill06
 /kill07
diff --git a/testcases/kernel/syscalls/kill/kill03.c b/testcases/kernel/syscalls/kill/kill03.c
index 5770ae0..21be09f 100644
--- a/testcases/kernel/syscalls/kill/kill03.c
+++ b/testcases/kernel/syscalls/kill/kill03.c
@@ -1,165 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
+ * 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 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
- *	kill03.c
- *
- * DESCRIPTION
- *	Test case to check that kill fails when given an invalid signal.
- *
- * ALGORITHM
- *	call setup
- *	loop if the -i option was given
- *	fork a child
- *	execute the kill system call with an invalid signal
- *	check the return value
- *	if return value is not -1
- *		issue a FAIL message, break remaining tests and cleanup
- *	if we are doing functional testing
- *		if the errno was set to 22 (invalid argument).
- *			issue a PASS message
- *		otherwise
- *			issue a FAIL message
- *	call cleanup
- *
- * USAGE
- *  kill03 [-c n] [-f] [-i n] [-I x] [-P x] [-t]
- *     where,  -c n : Run n copies concurrently.
- *             -f   : Turn off functionality Testing.
- *             -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.
+ * 1) kill() fails with errno set to EINVAL if given an invalid signal.
+ * 2) kill() fails with errno set to ESRCH if given a non-existent pid.
+ * 3) kill() fails with errno set to ESRCH if the given pid is INT_MIN.
  *
  * HISTORY
  *	07/2001 Ported by Wayne Boyer
- *
- * RESTRICTIONS
- *	none
  */
 
-#include "test.h"
-
+#include <sys/types.h>
 #include <signal.h>
-#include <errno.h>
-#include <sys/wait.h>
-
-void cleanup(void);
-void setup(void);
-void do_child(void);
-
-char *TCID = "kill03";
-int TST_TOTAL = 1;
-
-#define TEST_SIG 2000
-
-int main(int ac, char **av)
+#include <unistd.h>
+#include "tst_test.h"
+
+static pid_t real_pid, fake_pid, int_min_pid;
+static void cleanup(void);
+
+static struct tcase {
+	int test_sig;
+	int exp_errno;
+	int child_flag;
+	pid_t *pid;
+} tcases[] = {
+	{2000, EINVAL, 1, &real_pid},
+	{SIGKILL, ESRCH, 0, &fake_pid},
+	{SIGKILL, ESRCH, 0, &int_min_pid}
+};
+
+static void verify_kill(unsigned int n)
 {
-	int lc;
-	pid_t pid;
-	int exno, status;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-#ifdef UCLINUX
-	maybe_run_child(&do_child, "");
-#endif
+	struct tcase *tc = &tcases[n];
 
-	setup();
-
-	/* The following loop checks looping state if -i option given */
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		/* reset tst_count in case we are looping */
-		tst_count = 0;
-		status = 1;
-		exno = 1;
-		pid = FORK_OR_VFORK();
-		if (pid < 0) {
-			tst_brkm(TBROK, cleanup, "Fork of child failed");
-		} else if (pid == 0) {
-#ifdef UCLINUX
-			if (self_exec(av[0], "") < 0) {
-				tst_brkm(TBROK, cleanup,
-					 "self_exec of child failed");
-			}
-#else
-			do_child();
-#endif
-		} else {
-			TEST(kill(pid, TEST_SIG));
-			kill(pid, SIGKILL);
-			waitpid(pid, &status, 0);
-		}
+	if (tc->child_flag) {
+		real_pid = SAFE_FORK();
+		if (!real_pid)
+			pause();
+	}
+
+	TEST(kill(*tc->pid, tc->test_sig));
+	if (TST_RET != -1) {
+		tst_res(TFAIL, "kill should fail but not, return %ld", TST_RET);
+		return;
+	}
 
-		if (TEST_RETURN != -1) {
-			tst_brkm(TFAIL, cleanup, "%s failed - errno = %d : %s "
-				 "Expected a return value of -1 got %ld",
-				 TCID, TEST_ERRNO, strerror(TEST_ERRNO),
-				 TEST_RETURN);
-		}
+	if (tc->exp_errno == TST_ERR)
+		tst_res(TPASS | TTERRNO, "kill failed as expected");
+	else
+		tst_res(TFAIL | TTERRNO, "kill expected %s but got",
+			tst_strerrno(tc->exp_errno));
 
-		/*
-		 * Check to see if the errno was set to the expected
-		 * value of 22 : EINVAL.
-		 */
-		if (TEST_ERRNO == EINVAL) {
-			tst_resm(TPASS, "errno set to %d : %s, as "
-				 "expected", TEST_ERRNO,
-				 strerror(TEST_ERRNO));
-		} else {
-			tst_resm(TFAIL, "errno set to %d : %s expected "
-				 "%d : %s", TEST_ERRNO,
-				 strerror(TEST_ERRNO), 22,
-				 strerror(22));
-		}
+	if (tc->child_flag) {
+		cleanup();
+		real_pid = 0;
 	}
-
-	cleanup();
-	tst_exit();
 }
 
-/*
- * do_child()
- */
-void do_child(void)
+static void setup(void)
 {
-	int exno = 1;
-
-	pause();
-	exit(exno);
+	fake_pid = tst_get_unused_pid();
+	int_min_pid = INT_MIN;
 }
 
-/*
- * setup() - performs all ONE TIME setup for this test
- */
-void setup(void)
+static void cleanup(void)
 {
-
-	TEST_PAUSE;
+	if (real_pid > 0) {
+		SAFE_KILL(real_pid, SIGKILL);
+		SAFE_WAIT(NULL);
+	}
 }
 
-/*
- * cleanup() - performs all the ONE TIME cleanup for this test at completion
- * or premature exit.
- */
-void cleanup(void)
-{
-
-}
+static struct tst_test test = {
+	.test = verify_kill,
+	.tcnt = ARRAY_SIZE(tcases),
+	.setup = setup,
+	.cleanup = cleanup,
+	.forks_child = 1,
+};
diff --git a/testcases/kernel/syscalls/kill/kill04.c b/testcases/kernel/syscalls/kill/kill04.c
deleted file mode 100644
index f3bec13..0000000
--- a/testcases/kernel/syscalls/kill/kill04.c
+++ /dev/null
@@ -1,133 +0,0 @@
-/*
- *
- *   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 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
- *	kill04.c
- *
- * DESCRIPTION
- *	Test case to check that kill() fails when passed a non-existant pid.
- *
- * ALGORITHM
- *	call setup
- *	loop if the -i option was given
- *	fork a child
- *	execute the kill system call with a non-existant PID
- *	check the return value
- *	if return value is not -1
- *		issue a FAIL message, break remaining tests and cleanup
- *	if we are doing functional testing
- *		if the errno was set to 3 (No such proccess)
- *			issue a PASS message
- *		otherwise
- *			issue a FAIL message
- *	call cleanup
- *
- * USAGE
- *  kill04 [-c n] [-f] [-i n] [-I x] [-P x] [-t]
- *     where,  -c n : Run n copies concurrently.
- *             -f   : Turn off functionality Testing.
- *             -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.
- *
- * HISTORY
- *	07/2001 Ported by Wayne Boyer
- *
- * RESTRICTIONS
- *	This test should be run by a non-root user
- */
-
-#include "test.h"
-
-#include <signal.h>
-#include <errno.h>
-#include <sys/wait.h>
-
-void cleanup(void);
-void setup(void);
-void do_child(void);
-
-char *TCID = "kill04";
-int TST_TOTAL = 1;
-
-#define TEST_SIG SIGKILL
-
-int main(int ac, char **av)
-{
-	int lc;
-	pid_t fake_pid;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	/* The following loop checks looping state if -i option given */
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		/* reset tst_count in case we are looping */
-		tst_count = 0;
-
-		fake_pid = tst_get_unused_pid(cleanup);
-		TEST(kill(fake_pid, TEST_SIG));
-
-		if (TEST_RETURN != -1) {
-			tst_brkm(TFAIL, cleanup, "%s failed - errno = %d : %s "
-				 "Expected a return value of -1 got %ld",
-				 TCID, TEST_ERRNO, strerror(TEST_ERRNO),
-				 TEST_RETURN);
-		}
-
-		/*
-		 * Check to see if the errno was set to the expected
-		 * value of 3 : ESRCH
-		 */
-		if (TEST_ERRNO == ESRCH) {
-			tst_resm(TPASS, "errno set to %d : %s, as "
-				 "expected", TEST_ERRNO,
-				 strerror(TEST_ERRNO));
-		} else {
-			tst_resm(TFAIL, "errno set to %d : %s expected "
-				 "%d : %s", TEST_ERRNO,
-				 strerror(TEST_ERRNO), 3, strerror(3));
-		}
-	}
-
-	cleanup();
-	tst_exit();
-}
-
-/*
- * setup() - performs all ONE TIME setup for this test
- */
-void setup(void)
-{
-
-	TEST_PAUSE;
-}
-
-/*
- * cleanup() - performs all the ONE TIME cleanup for this test at completion
- * or premature exit.
- */
-void cleanup(void)
-{
-
-}
-- 
1.8.3.1




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

* [LTP] [PATCH 3/4] syscalls/kill05:Cleanup && Convert to new library
  2020-08-20 10:26 [LTP] [PATCH 1/4] syscalls/kill01: Remove it Feiyu Zhu
  2020-08-20 10:26 ` [LTP] [PATCH 2/4] syscalls/kill03, 04: Cleanup && Convert to new library Feiyu Zhu
@ 2020-08-20 10:26 ` Feiyu Zhu
  2020-08-21  5:49   ` Li Wang
  2020-08-20 10:26 ` [LTP] [PATCH 4/4] syscalls/kill06:Cleanup " Feiyu Zhu
  2020-08-20 13:22 ` [LTP] [PATCH 1/4] syscalls/kill01: Remove it Cyril Hrubis
  3 siblings, 1 reply; 14+ messages in thread
From: Feiyu Zhu @ 2020-08-20 10:26 UTC (permalink / raw)
  To: ltp

1) Take use of some safe macros

Reviewed-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
Signed-off-by: Feiyu Zhu <zhufy.jy@cn.fujitsu.com>
---
 testcases/kernel/syscalls/kill/Makefile |   4 +-
 testcases/kernel/syscalls/kill/kill05.c | 243 ++++++++------------------------
 2 files changed, 58 insertions(+), 189 deletions(-)

diff --git a/testcases/kernel/syscalls/kill/Makefile b/testcases/kernel/syscalls/kill/Makefile
index 3d1b146..0cc064b 100644
--- a/testcases/kernel/syscalls/kill/Makefile
+++ b/testcases/kernel/syscalls/kill/Makefile
@@ -3,11 +3,11 @@
 
 top_srcdir		?= ../../../..
 
-LTPLIBS = ltpipc
+LTPLIBS = ltpipc ltpnewipc
 
 include $(top_srcdir)/include/mk/testcases.mk
 
 kill07: LTPLDLIBS  = -lltpipc
-kill05: LTPLDLIBS  = -lltpipc
+kill05: LTPLDLIBS  = -lltpnewipc
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/kill/kill05.c b/testcases/kernel/syscalls/kill/kill05.c
index ccef5af..039d510 100644
--- a/testcases/kernel/syscalls/kill/kill05.c
+++ b/testcases/kernel/syscalls/kill/kill05.c
@@ -1,56 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
+ * 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 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
- *	kill05.c
- *
- * DESCRIPTION
- *	Test case to check that kill() fails when passed a pid owned by another
- *	user.
- *
- * ALGORITHM
- *	call setup
- *	loop if the -i option was given
- *	setup a shared memory segment to for a flag which will notify
- *	ltpuser1's process that life is not worth living in a continuous loop.
- *	fork a child and set the euid to ltpuser1
- *	set the parents euid to ltpuser2
- *	execute the kill system call on ltpuser1's pid
- *	check the return value
- *	if return value is not -1
- *		issue a FAIL message, break remaining tests and cleanup
- *      if we are doing functional testing
- *              if the errno was set to 1 (Operation not permitted)
- *                      issue a PASS message
- *              otherwise
- *                      issue a FAIL message
- *	call cleanup
- *
- * USAGE
- *  kill05 [-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.
+ * Test case to check that kill() fails when passed a pid owned by another user.
  *
  * HISTORY
  *	07/2001 Ported by Wayne Boyer
@@ -66,67 +18,20 @@
  *	Looping with the -i option does not work correctly.
  */
 
-#include <sys/types.h>
-#include <sys/ipc.h>
-#include <sys/shm.h>
 #include <sys/wait.h>
-#include <errno.h>
 #include <pwd.h>
-#include <signal.h>
-#include <string.h>
-#include <stdio.h>
 #include <stdlib.h>
-#include <unistd.h>
-
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_test.h"
+#include "libnewipc.h"
+#include "tst_safe_sysv_ipc.h"
+#include "tst_safe_macros.h"
 
-extern void rm_shm(int);
-
-void cleanup(void);
-void setup(void);
-void do_child(void);
-void do_master_child(char **av);
-
-char *TCID = "kill05";
-int TST_TOTAL = 1;
-int shmid1 = -1;
-extern key_t semkey;
+static uid_t nobody_uid, bin_uid;
 int *flag;
+static int shm_id = -1;
+static key_t shm_key;
 
-extern int getipckey();
-
-#define TEST_SIG SIGKILL
-
-int main(int ac, char **av)
-{
-	pid_t pid;
-	int status;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-#ifdef UCLINUX
-	maybe_run_child(&do_child, "");
-#endif
-
-	setup();		/* global setup */
-
-	pid = FORK_OR_VFORK();
-	if (pid == -1)
-		tst_brkm(TBROK, cleanup, "Fork failed");
-	else if (pid == 0)
-		do_master_child(av);
-
-	if (waitpid(pid, &status, 0) == -1)
-		tst_resm(TBROK | TERRNO, "waitpid failed");
-	else if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
-		tst_resm(TFAIL, "child exited abnormally");
-	else
-		tst_resm(TPASS, "received expected errno(EPERM)");
-	cleanup();
-	tst_exit();
-}
-
-void wait_for_flag(int value)
+static void wait_for_flag(int value)
 {
 	while (1) {
 		if (*flag == value)
@@ -136,107 +41,71 @@ void wait_for_flag(int value)
 	}
 }
 
-/*
- * do_master_child()
- */
-void do_master_child(char **av)
+void do_master_child(void)
 {
 	pid_t pid1;
-	int status;
-
-	char user1name[] = "nobody";
-	char user2name[] = "bin";
-
-	struct passwd *ltpuser1, *ltpuser2;
-
-	tst_count = 0;
 
 	*flag = 0;
-
-	pid1 = FORK_OR_VFORK();
-
-	if (pid1 == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "Fork failed");
-
+	pid1 = SAFE_FORK();
 	if (pid1 == 0) {
-		ltpuser1 = SAFE_GETPWNAM(NULL, user1name);
-		if (setreuid(ltpuser1->pw_uid, ltpuser1->pw_uid) == -1) {
-			perror("setreuid failed (in child)");
-			exit(1);
-		}
+		SAFE_SETREUID(nobody_uid, nobody_uid);
 		*flag = 1;
-#ifdef UCLINUX
-		if (self_exec(av[0], "") < 0) {
-			perror("self_exec failed");
-			exit(1);
-		}
-#else
-		do_child();
-#endif
-	}
-	ltpuser2 = SAFE_GETPWNAM(NULL, user2name);
-	if (setreuid(ltpuser2->pw_uid, ltpuser2->pw_uid) == -1) {
-		perror("seteuid failed");
-		exit(1);
+		wait_for_flag(2);
+		exit(0);
 	}
-
-	/* wait until child sets its euid */
+	SAFE_SETREUID(bin_uid, bin_uid);
 	wait_for_flag(1);
-
-	TEST(kill(pid1, TEST_SIG));
-
-	/* signal the child that we're done */
+	TEST(kill(pid1, SIGKILL));
 	*flag = 2;
+	SAFE_WAITPID(pid1, NULL, 0);
 
-	if (waitpid(pid1, &status, 0) == -1) {
-		perror("waitpid failed");
-		exit(1);
+	if (TST_RET == 0) {
+		tst_res(TFAIL, "kill succeeded unexpectedly");
+		return;
 	}
 
-	if (TEST_RETURN != -1) {
-		printf("kill succeeded unexpectedly\n");
-		exit(1);
-	}
+	if (TST_ERR == EPERM)
+		tst_res(TPASS, "kill failed with EPERM");
+	else
+		tst_res(TFAIL | TTERRNO, "kill failed expected EPERM, but got");
+}
 
-	/*
-	 * Check to see if the errno was set to the expected
-	 * value of 1 : EPERM
-	 */
-	if (TEST_ERRNO == EPERM) {
-		printf("kill failed with EPERM\n");
+static void verify_kill(void)
+{
+	pid_t pid;
+
+	pid = SAFE_FORK();
+	if (pid == 0) {
+		do_master_child();
 		exit(0);
 	}
-	perror("kill failed unexpectedly");
-	exit(1);
-}
 
-void do_child(void)
-{
-	wait_for_flag(2);
-	exit(0);
+	tst_reap_children();
 }
 
-void setup(void)
+static void setup(void)
 {
-	tst_require_root();
-
-	TEST_PAUSE;
-
-	tst_tmpdir();
-
-	semkey = getipckey();
-
-	if ((shmid1 = shmget(semkey, getpagesize(), 0666 | IPC_CREAT)) == -1)
-		tst_brkm(TBROK, cleanup, "Failed to setup shared memory");
-
-	if ((flag = shmat(shmid1, 0, 0)) == (int *)-1)
-		tst_brkm(TBROK | TERRNO, cleanup,
-			 "Failed to attach shared memory:%d", shmid1);
+	struct passwd *pw;
+	shm_key = GETIPCKEY();
+	shm_id = SAFE_SHMGET(shm_key, getpagesize(), 0666 | IPC_CREAT);
+	flag = SAFE_SHMAT(shm_id, 0, 0);
+	pw = SAFE_GETPWNAM("nobody");
+	nobody_uid = pw->pw_uid;
+	pw = SAFE_GETPWNAM("bin");
+	bin_uid = pw->pw_uid;
 }
 
-void cleanup(void)
+static void cleanup(void)
 {
-	rm_shm(shmid1);
-
-	tst_rmdir();
+	if (shm_id != -1)
+		SAFE_SHMCTL(shm_id, IPC_RMID, NULL);
 }
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = verify_kill,
+	.needs_tmpdir = 1,
+	.needs_root = 1,
+	.forks_child = 1,
+};
-- 
1.8.3.1




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

* [LTP] [PATCH 4/4] syscalls/kill06:Cleanup && Convert to new library
  2020-08-20 10:26 [LTP] [PATCH 1/4] syscalls/kill01: Remove it Feiyu Zhu
  2020-08-20 10:26 ` [LTP] [PATCH 2/4] syscalls/kill03, 04: Cleanup && Convert to new library Feiyu Zhu
  2020-08-20 10:26 ` [LTP] [PATCH 3/4] syscalls/kill05:Cleanup " Feiyu Zhu
@ 2020-08-20 10:26 ` Feiyu Zhu
  2020-08-21  6:57   ` Li Wang
  2020-08-20 13:22 ` [LTP] [PATCH 1/4] syscalls/kill01: Remove it Cyril Hrubis
  3 siblings, 1 reply; 14+ messages in thread
From: Feiyu Zhu @ 2020-08-20 10:26 UTC (permalink / raw)
  To: ltp

1) Take use of some safe macros

Reviewed-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
Signed-off-by: Feiyu Zhu <zhufy.jy@cn.fujitsu.com>
---
 testcases/kernel/syscalls/kill/kill06.c | 208 ++++++--------------------------
 1 file changed, 37 insertions(+), 171 deletions(-)

diff --git a/testcases/kernel/syscalls/kill/kill06.c b/testcases/kernel/syscalls/kill/kill06.c
index 7a1e8bf..8fcc8bc 100644
--- a/testcases/kernel/syscalls/kill/kill06.c
+++ b/testcases/kernel/syscalls/kill/kill06.c
@@ -1,187 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
+ * 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 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
- *	kill06.c
- *
- * DESCRIPTION
- *	Test case to check the basic functionality of kill() when killing an
- *	entire process group with a negative pid.
- *
- * ALGORITHM
- *	call setup
- *	loop if the -i option was given
- *	fork 5 children
- *	execute the kill system call
- *	check the return value
- *	if return value is -1
- *		issue a FAIL message, break remaining tests and cleanup
- *	if we are doing functional testing
- *		if the processes were terminated with the expected signal.
- *			issue a PASS message
- *		otherwise
- *			issue a FAIL message
- *	call cleanup
- *
- * USAGE
- *  kill06 [-c n] [-f] [-i n] [-I x] [-P x] [-t]
- *     where,  -c n : Run n copies concurrently.
- *             -f   : Turn off functionality Testing.
- *             -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.
+ * Test case to check the basic functionality of kill() when killing an
+ * entire process group with a negative pid.
  *
  * HISTORY
  *	07/2001 Ported by Wayne Boyer
- *
- * RESTRICTIONS
- *	This test should be run as a non-root user.
  */
 
-#include "test.h"
-
+#include <sys/types.h>
 #include <signal.h>
-#include <errno.h>
 #include <sys/wait.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include "tst_test.h"
 
-void cleanup(void);
-void setup(void);
-void do_child(void);
-
-char *TCID = "kill06";
-int TST_TOTAL = 1;
-
-#define TEST_SIG SIGKILL
-
-int main(int ac, char **av)
+static void verify_kill(void)
 {
-	int lc;
-	pid_t pid1, pid2;
-	int exno, status, nsig, i;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-#ifdef UCLINUX
-	maybe_run_child(&do_child, "");
-#endif
-
-	setup();		/* global setup */
-
-	/* The following loop checks looping state if -i option given */
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		/* reset tst_count in case we are looping */
-		tst_count = 0;
-		status = 1;
-		exno = 1;
-
-		/* Fork a process and set the process group so that */
-		/* it is different from this one.  Fork 5 more children. */
-
-		pid1 = FORK_OR_VFORK();
-		if (pid1 < 0) {
-			tst_brkm(TBROK, cleanup, "Fork of first child failed");
-		} else if (pid1 == 0) {
-			setpgrp();
-			for (i = 0; i < 5; i++) {
-				pid2 = FORK_OR_VFORK();
-				if (pid2 < 0) {
-					tst_brkm(TBROK, cleanup, "Fork failed");
-				} else if (pid2 == 0) {
-#ifdef UCLINUX
-					if (self_exec(av[0], "") < 0) {
-						tst_brkm(TBROK, cleanup,
-							 "self_exec of "
-							 "child failed");
-					}
-#else
-					do_child();
-#endif
-				}
-			}
-			/* Kill all processes in this process group */
-			TEST(kill(-getpgrp(), TEST_SIG));
-			sleep(300);
-
-			tst_resm(TINFO, "%d never received a"
-				 " signal", getpid());
-			exit(exno);
-		} else {
-			waitpid(pid1, &status, 0);
-			if (TEST_RETURN != 0) {
-				tst_brkm(TFAIL, cleanup, "%s failed - errno = "
-					 "%d : %s", TCID, TEST_ERRNO,
-					 strerror(TEST_ERRNO));
-			}
+	pid_t pid, child_pid[5];
+	int nsig, i, status;
+
+	pid = SAFE_FORK();
+	if (pid == 0) {
+		setpgrp();
+		for (i = 0; i < 5; i++) {
+			child_pid[i] = SAFE_FORK();
+			if (!child_pid[i])
+				pause();
 		}
 
-		/*
-		 * Check to see if the process was terminated with the
-		 * expected signal.
-		 */
-		nsig = WTERMSIG(status);
-		if (!nsig) {
-			tst_resm(TFAIL, "Did not receive any signal");
-		} else if (nsig == TEST_SIG) {
-			tst_resm(TPASS, "received expected signal %d",
-				 nsig);
-		} else {
-			tst_resm(TFAIL,
-				 "expected signal %d received %d",
-				 TEST_SIG, nsig);
-		}
+		TEST(kill(-getpgrp(), SIGKILL));
+		if (TST_RET != 0)
+			tst_res(TFAIL | TTERRNO, "kill failed");
+		tst_res(TINFO, "%d never received a signal", getpid());
+		exit(0);
 	}
-
-	cleanup();
-	tst_exit();
-}
-
-/*
- * do_child()
- */
-void do_child(void)
-{
-	int exno = 1;
-
-	sleep(299);
-
-	tst_resm(TINFO, "%d never received a" " signal", getpid());
-	exit(exno);
-}
-
-/*
- * setup() - performs all ONE TIME setup for this test
- */
-void setup(void)
-{
-	/* Setup default signal handling */
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
+
+	SAFE_WAITPID(pid, &status, 0);
+	nsig = WTERMSIG(status);
+	if (nsig != SIGKILL) {
+		tst_res(TFAIL, "wait: unexpected signal %d returned, "
+			"expected SIGKILL(9)", nsig);
+		return;
+	}
+	tst_res(TPASS, "receive expected signal SIGKILL(9)");
 }
 
-/*
- * cleanup() - performs all the ONE TIME cleanup for this test@completion
- * or premature exit.
- */
-void cleanup(void)
-{
-
-}
+static struct tst_test test = {
+	.forks_child = 1,
+	.test_all = verify_kill,
+};
-- 
1.8.3.1




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

* [LTP] [PATCH 2/4] syscalls/kill03, 04: Cleanup && Convert to new library
  2020-08-20 10:26 ` [LTP] [PATCH 2/4] syscalls/kill03, 04: Cleanup && Convert to new library Feiyu Zhu
@ 2020-08-20 12:18   ` Li Wang
  2020-08-21  1:18     ` Yang Xu
  0 siblings, 1 reply; 14+ messages in thread
From: Li Wang @ 2020-08-20 12:18 UTC (permalink / raw)
  To: ltp

Feiyu Zhu <zhufy.jy@cn.fujitsu.com> wrote:

...
> +#include <unistd.h>
> +#include "tst_test.h"
> +
> +static pid_t real_pid, fake_pid, int_min_pid;
> +static void cleanup(void);
> +
> +static struct tcase {
> +       int test_sig;
> +       int exp_errno;
> +       int child_flag;
>

The child_flag field is not necessary, we could prepare a real child in
setup()
and reclaim it after testing via check the real_pid value, that will be
more easily.

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200820/2115494a/attachment.htm>

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

* [LTP] [PATCH 1/4] syscalls/kill01: Remove it
  2020-08-20 10:26 [LTP] [PATCH 1/4] syscalls/kill01: Remove it Feiyu Zhu
                   ` (2 preceding siblings ...)
  2020-08-20 10:26 ` [LTP] [PATCH 4/4] syscalls/kill06:Cleanup " Feiyu Zhu
@ 2020-08-20 13:22 ` Cyril Hrubis
  3 siblings, 0 replies; 14+ messages in thread
From: Cyril Hrubis @ 2020-08-20 13:22 UTC (permalink / raw)
  To: ltp

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 2/4] syscalls/kill03, 04: Cleanup && Convert to new library
  2020-08-20 12:18   ` Li Wang
@ 2020-08-21  1:18     ` Yang Xu
  2020-08-21  1:56       ` Li Wang
  0 siblings, 1 reply; 14+ messages in thread
From: Yang Xu @ 2020-08-21  1:18 UTC (permalink / raw)
  To: ltp

Hi Li


> 
> Feiyu Zhu <zhufy.jy@cn.fujitsu.com <mailto:zhufy.jy@cn.fujitsu.com>> wrote:
> 
>     ...
>     +#include <unistd.h>
>     +#include "tst_test.h"
>     +
>     +static pid_t real_pid, fake_pid, int_min_pid;
>     +static void cleanup(void);
>     +
>     +static struct tcase {
>     +? ? ? ?int test_sig;
>     +? ? ? ?int exp_errno;
>     +? ? ? ?int child_flag;
> 
> 
> The child_flag field is not necessary, we could prepare a real child in 
> setup()
> and?reclaim it after testing via check the real_pid value, that will be 
> more easily.
When I reviewed this patch in internal, I had the same idea. But when I 
try it and this case will hang because sub test will wait children 
finished by using tst_reap_childrens below:

lib/tst_test.c
static void run_tests(void)
{
	...
	 for (i = 0; i < tst_test->tcnt; i++) {
                 saved_results = *results;
                 tst_test->test(i);

                 if (getpid() != main_pid) {
                         exit(0);
                 }

                 tst_reap_children();

                 if (results_equal(&saved_results, results))
                         tst_brk(TBROK, "Test %i haven't reported 
results!", i);
         }


}

Also, we can use the current process id but it may has unexpected result 
when kill succeed. So fork a child to test maybe a better solution.
> 
> -- 
> Regards,
> Li Wang
> 
> 



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

* [LTP] [PATCH 2/4] syscalls/kill03, 04: Cleanup && Convert to new library
  2020-08-21  1:18     ` Yang Xu
@ 2020-08-21  1:56       ` Li Wang
  2020-08-21  2:21         ` Yang Xu
  0 siblings, 1 reply; 14+ messages in thread
From: Li Wang @ 2020-08-21  1:56 UTC (permalink / raw)
  To: ltp

On Fri, Aug 21, 2020 at 9:18 AM Yang Xu <xuyang2018.jy@cn.fujitsu.com>
wrote:

> Hi Li
>
>
> >
> > Feiyu Zhu <zhufy.jy@cn.fujitsu.com <mailto:zhufy.jy@cn.fujitsu.com>>
> wrote:
> >
> >     ...
> >     +#include <unistd.h>
> >     +#include "tst_test.h"
> >     +
> >     +static pid_t real_pid, fake_pid, int_min_pid;
> >     +static void cleanup(void);
> >     +
> >     +static struct tcase {
> >     +       int test_sig;
> >     +       int exp_errno;
> >     +       int child_flag;
> >
> >
> > The child_flag field is not necessary, we could prepare a real child in
> > setup()
> > and reclaim it after testing via check the real_pid value, that will be
> > more easily.
> When I reviewed this patch in internal, I had the same idea. But when I
> try it and this case will hang because sub test will wait children
> finished by using tst_reap_childrens below:
>
> lib/tst_test.c
> static void run_tests(void)
> {
>         ...
>          for (i = 0; i < tst_test->tcnt; i++) {
>                  saved_results = *results;
>                  tst_test->test(i);
>
>                  if (getpid() != main_pid) {
>                          exit(0);
>                  }
>
>                  tst_reap_children();
>
>                  if (results_equal(&saved_results, results))
>                          tst_brk(TBROK, "Test %i haven't reported
> results!", i);
>          }
>
>
> }
>
> Also, we can use the current process id but it may has unexpected result
> when kill succeed. So fork a child to test maybe a better solution.
>

Hmm, sorry for the uncleared description, actually I meant, to use real_pid
instead of the tc->child_flag directly, then start to reclaim the child
when the
real_pid test finishing.

Does this below diff work for you?

--- a/testcases/kernel/syscalls/kill/kill03.c
+++ b/testcases/kernel/syscalls/kill/kill03.c
@@ -21,24 +21,17 @@ static void cleanup(void);
 static struct tcase {
        int test_sig;
        int exp_errno;
-       int child_flag;
        pid_t *pid;
 } tcases[] = {
-       {2000, EINVAL, 1, &real_pid},
-       {SIGKILL, ESRCH, 0, &fake_pid},
-       {SIGKILL, ESRCH, 0, &int_min_pid}
+       {2000, EINVAL, &real_pid},
+       {SIGKILL, ESRCH, &fake_pid},
+       {SIGKILL, ESRCH, &int_min_pid}
 };

 static void verify_kill(unsigned int n)
 {
        struct tcase *tc = &tcases[n];

-       if (tc->child_flag) {
-               real_pid = SAFE_FORK();
-               if (!real_pid)
-                       pause();
-       }
-
        TEST(kill(*tc->pid, tc->test_sig));
        if (TST_RET != -1) {
                tst_res(TFAIL, "kill should fail but not, return %ld",
TST_RET);
@@ -51,14 +44,19 @@ static void verify_kill(unsigned int n)
                tst_res(TFAIL | TTERRNO, "kill expected %s but got",
                        tst_strerrno(tc->exp_errno));

-       if (tc->child_flag) {
+       if (real_pid) {
                cleanup();
                real_pid = 0;
        }
+
 }

 static void setup(void)
 {
+       real_pid = SAFE_FORK();
+       if (!real_pid)
+               pause();
+
        fake_pid = tst_get_unused_pid();
        int_min_pid = INT_MIN;
 }
-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200821/9d88dd10/attachment-0001.htm>

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

* [LTP] [PATCH 2/4] syscalls/kill03, 04: Cleanup && Convert to new library
  2020-08-21  1:56       ` Li Wang
@ 2020-08-21  2:21         ` Yang Xu
  2020-08-21  3:16           ` Li Wang
  0 siblings, 1 reply; 14+ messages in thread
From: Yang Xu @ 2020-08-21  2:21 UTC (permalink / raw)
  To: ltp

Hi Li


> 
> 
> On Fri, Aug 21, 2020 at 9:18 AM Yang Xu <xuyang2018.jy@cn.fujitsu.com 
> <mailto:xuyang2018.jy@cn.fujitsu.com>> wrote:
> 
>     Hi Li
> 
> 
>      >
>      > Feiyu Zhu <zhufy.jy@cn.fujitsu.com
>     <mailto:zhufy.jy@cn.fujitsu.com> <mailto:zhufy.jy@cn.fujitsu.com
>     <mailto:zhufy.jy@cn.fujitsu.com>>> wrote:
>      >
>      >? ? ?...
>      >? ? ?+#include <unistd.h>
>      >? ? ?+#include "tst_test.h"
>      >? ? ?+
>      >? ? ?+static pid_t real_pid, fake_pid, int_min_pid;
>      >? ? ?+static void cleanup(void);
>      >? ? ?+
>      >? ? ?+static struct tcase {
>      >? ? ?+? ? ? ?int test_sig;
>      >? ? ?+? ? ? ?int exp_errno;
>      >? ? ?+? ? ? ?int child_flag;
>      >
>      >
>      > The child_flag field is not necessary, we could prepare a real
>     child in
>      > setup()
>      > and?reclaim it after testing via check the real_pid value, that
>     will be
>      > more easily.
>     When I reviewed this patch in internal, I had the same idea. But when I
>     try it and this case will hang because sub test will wait children
>     finished by using tst_reap_childrens below:
> 
>     lib/tst_test.c
>     static void run_tests(void)
>     {
>      ? ? ? ? ...
>      ? ? ? ? ?for (i = 0; i < tst_test->tcnt; i++) {
>      ? ? ? ? ? ? ? ? ?saved_results = *results;
>      ? ? ? ? ? ? ? ? ?tst_test->test(i);
> 
>      ? ? ? ? ? ? ? ? ?if (getpid() != main_pid) {
>      ? ? ? ? ? ? ? ? ? ? ? ? ?exit(0);
>      ? ? ? ? ? ? ? ? ?}
> 
>      ? ? ? ? ? ? ? ? ?tst_reap_children();
> 
>      ? ? ? ? ? ? ? ? ?if (results_equal(&saved_results, results))
>      ? ? ? ? ? ? ? ? ? ? ? ? ?tst_brk(TBROK, "Test %i haven't reported
>     results!", i);
>      ? ? ? ? ?}
> 
> 
>     }
> 
>     Also, we can use the current process id but it may has unexpected
>     result
>     when kill succeed. So fork a child to test maybe a better solution.
> 
> 
> Hmm, sorry for the uncleared description, actually I meant, to use real_pid
> instead of the?tc->child_flag directly, then start to reclaim the child 
> when the
> real_pid test finishing.
> 
> Does this below diff work for you?
It looks well. But the real_pid only valid when the first sub test and 
the real pid is equals to 0 when using -i parameters because we have 
killed this children.

For pid = 0, it means  then sig is sent to every process in the process 
group of the calling process.  So it looks like we test different thing 
when using -i parameters.  What do you think about this?
> 
> --- a/testcases/kernel/syscalls/kill/kill03.c
> +++ b/testcases/kernel/syscalls/kill/kill03.c
> @@ -21,24 +21,17 @@ static void cleanup(void);
>  ?static struct tcase {
>  ? ? ? ? int test_sig;
>  ? ? ? ? int exp_errno;
> - ? ? ? int child_flag;
>  ? ? ? ? pid_t *pid;
>  ?} tcases[] = {
> - ? ? ? {2000, EINVAL, 1, &real_pid},
> - ? ? ? {SIGKILL, ESRCH, 0, &fake_pid},
> - ? ? ? {SIGKILL, ESRCH, 0, &int_min_pid}
> + ? ? ? {2000, EINVAL, &real_pid},
> + ? ? ? {SIGKILL, ESRCH, &fake_pid},
> + ? ? ? {SIGKILL, ESRCH, &int_min_pid}
>  ?};
> 
>  ?static void verify_kill(unsigned int n)
>  ?{
>  ? ? ? ? struct tcase *tc = &tcases[n];
> 
> - ? ? ? if (tc->child_flag) {
> - ? ? ? ? ? ? ? real_pid = SAFE_FORK();
> - ? ? ? ? ? ? ? if (!real_pid)
> - ? ? ? ? ? ? ? ? ? ? ? pause();
> - ? ? ? }
> -
>  ? ? ? ? TEST(kill(*tc->pid, tc->test_sig));
>  ? ? ? ? if (TST_RET != -1) {
>  ? ? ? ? ? ? ? ? tst_res(TFAIL, "kill should fail but not, return %ld", 
> TST_RET);
> @@ -51,14 +44,19 @@ static void verify_kill(unsigned int n)
>  ? ? ? ? ? ? ? ? tst_res(TFAIL | TTERRNO, "kill expected %s but got",
>  ? ? ? ? ? ? ? ? ? ? ? ? tst_strerrno(tc->exp_errno));
> 
> - ? ? ? if (tc->child_flag) {
> + ? ? ? if (real_pid) {
>  ? ? ? ? ? ? ? ? cleanup();
>  ? ? ? ? ? ? ? ? real_pid = 0;
>  ? ? ? ? }
> +
>  ?}
> 
>  ?static void setup(void)
>  ?{
> + ? ? ? real_pid = SAFE_FORK();
> + ? ? ? if (!real_pid)
> + ? ? ? ? ? ? ? pause();
> +
>  ? ? ? ? fake_pid = tst_get_unused_pid();
>  ? ? ? ? int_min_pid = INT_MIN;
>  ?}
> -- 
> Regards,
> Li Wang



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

* [LTP] [PATCH 2/4] syscalls/kill03, 04: Cleanup && Convert to new library
  2020-08-21  2:21         ` Yang Xu
@ 2020-08-21  3:16           ` Li Wang
  2020-08-21  3:34             ` Yang Xu
  0 siblings, 1 reply; 14+ messages in thread
From: Li Wang @ 2020-08-21  3:16 UTC (permalink / raw)
  To: ltp

On Fri, Aug 21, 2020 at 10:22 AM Yang Xu <xuyang2018.jy@cn.fujitsu.com>
wrote:

> ...
> >     Also, we can use the current process id but it may has unexpected
> >     result
> >     when kill succeed. So fork a child to test maybe a better solution.
> >
> >
> > Hmm, sorry for the uncleared description, actually I meant, to use
> real_pid
> > instead of the tc->child_flag directly, then start to reclaim the child
> > when the
> > real_pid test finishing.
> >
> > Does this below diff work for you?
> It looks well. But the real_pid only valid when the first sub test and
> the real pid is equals to 0 when using -i parameters because we have
> killed this children.
>

Yes, you're right.
How about moving up the real_pid creator to the main process? does it work
for you?

static void verify_kill(unsigned int n)
{
        if (!real_pid) {
                real_pid = SAFE_FORK();
                if (!real_pid)
                        pause();
        }

        TEST(kill(*tc->pid, tc->test_sig));
        ...

        if (real_pid) {
                cleanup();
                real_pid = 0;
        }

}


>
> For pid = 0, it means  then sig is sent to every process in the process
> group of the calling process.  So it looks like we test different thing
> when using -i parameters.  What do you think about this?
>

I even think it is a good idea for code simplification:). In this case, the
first
the subtest is just to verify invalid signal for kill(), it doesn't matter
to go with
kill(0, invalid_signal), that only tries to kill the current main process.
isn't it?

If we go this way, not only the tc->child_flag is no needed, but also not
necessary
to fork a new child to be killed.

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200821/57245fb5/attachment-0001.htm>

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

* [LTP] [PATCH 2/4] syscalls/kill03, 04: Cleanup && Convert to new library
  2020-08-21  3:16           ` Li Wang
@ 2020-08-21  3:34             ` Yang Xu
  2020-08-21  3:43               ` Li Wang
  0 siblings, 1 reply; 14+ messages in thread
From: Yang Xu @ 2020-08-21  3:34 UTC (permalink / raw)
  To: ltp

Hi Li
> 
> 
> On Fri, Aug 21, 2020 at 10:22 AM Yang Xu <xuyang2018.jy@cn.fujitsu.com 
> <mailto:xuyang2018.jy@cn.fujitsu.com>> wrote:
> 
>     ...
>      >? ? ?Also, we can use the current process id but it may has unexpected
>      >? ? ?result
>      >? ? ?when kill succeed. So fork a child to test maybe a better
>     solution.
>      >
>      >
>      > Hmm, sorry for the uncleared description, actually I meant, to
>     use real_pid
>      > instead of the?tc->child_flag directly, then start to reclaim the
>     child
>      > when the
>      > real_pid test finishing.
>      >
>      > Does this below diff work for you?
>     It looks well. But the real_pid only valid when the first sub test and
>     the real pid is equals to 0 when using -i parameters because we have
>     killed this children.
> 
> 
> Yes, you're right.
> How about moving up the real_pid creator to the main process? does it 
> work for you?
> 
> static void verify_kill(unsigned int n)
> {
>  ? ? ? ? if (!real_pid) {
>  ? ? ? ? ? ? ? ? real_pid = SAFE_FORK();
>  ? ? ? ? ? ? ? ? if (!real_pid)
>  ? ? ? ? ? ? ? ? ? ? ? ? pause();
>  ? ? ? ? }
> 
>  ? ? ? ? TEST(kill(*tc->pid, tc->test_sig));
> ...
> 
>  ? ? ? ? if (real_pid) {
>  ? ? ? ? ? ? ? ? cleanup();
>  ? ? ? ? ? ? ? ? real_pid = 0;
>  ? ? ? ? }
> 
> }
> 
Yes, it looks ok,
> 
>     For pid = 0, it means? then sig is sent to every process in the process
>     group of the calling process.? So it looks like we test different thing
>     when using -i parameters.? What do you think about this?
> 
> 
> I even think it is a good idea for code simplification:). In this case, 
> the first
> the subtest is just to verify invalid signal for kill(), it doesn't 
> matter to go with
> kill(0, invalid_signal), that only tries to kill the current main 
> process. isn't it?
Yes. I think using current process id instead of 0 is more easier 
because the invalid signal can't kill process forever.

static void setup(void)
{
         real_pid = getpid();
         fake_pid = tst_get_unused_pid();
         int_min_pid = INT_MIN;
}

> 
> If we go this way, not only the?tc->child_flag is no needed, but also 
> not necessary
> to fork a new child to be killed.
> 
> -- 
> Regards,
> Li Wang



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

* [LTP] [PATCH 2/4] syscalls/kill03, 04: Cleanup && Convert to new library
  2020-08-21  3:34             ` Yang Xu
@ 2020-08-21  3:43               ` Li Wang
  0 siblings, 0 replies; 14+ messages in thread
From: Li Wang @ 2020-08-21  3:43 UTC (permalink / raw)
  To: ltp

On Fri, Aug 21, 2020 at 11:34 AM Yang Xu <xuyang2018.jy@cn.fujitsu.com>
wrote:

> ...
> Yes. I think using current process id instead of 0 is more easier
> because the invalid signal can't kill process forever.
>
> static void setup(void)
> {
>          real_pid = getpid();
>

Agreed!


-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200821/1cb95f23/attachment.htm>

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

* [LTP] [PATCH 3/4] syscalls/kill05:Cleanup && Convert to new library
  2020-08-20 10:26 ` [LTP] [PATCH 3/4] syscalls/kill05:Cleanup " Feiyu Zhu
@ 2020-08-21  5:49   ` Li Wang
  0 siblings, 0 replies; 14+ messages in thread
From: Li Wang @ 2020-08-21  5:49 UTC (permalink / raw)
  To: ltp

On Thu, Aug 20, 2020 at 6:27 PM Feiyu Zhu <zhufy.jy@cn.fujitsu.com> wrote:

> ...
>   *     Looping with the -i option does not work correctly.
>

This line should be removed since it works well with -i.

> +static void wait_for_flag(int value)
>  {
>         while (1) {
>                 if (*flag == value)
>

Sleep 1sec is too long here, we can use usleep(100) for a replacement.

+static struct tst_test test = {
> +       .setup = setup,
> +       .cleanup = cleanup,
> +       .test_all = verify_kill,
> +       .needs_tmpdir = 1,
>

'.needs_tmdir' is redundant.


> +       .needs_root = 1,
> +       .forks_child = 1,
> +};
>
The remaining part looks good.

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200821/d8fb8c0e/attachment.htm>

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

* [LTP] [PATCH 4/4] syscalls/kill06:Cleanup && Convert to new library
  2020-08-20 10:26 ` [LTP] [PATCH 4/4] syscalls/kill06:Cleanup " Feiyu Zhu
@ 2020-08-21  6:57   ` Li Wang
  0 siblings, 0 replies; 14+ messages in thread
From: Li Wang @ 2020-08-21  6:57 UTC (permalink / raw)
  To: ltp

Feiyu Zhu <zhufy.jy@cn.fujitsu.com> wrote:

...
> +       pid_t pid, child_pid[5];
>

I'd suggest dropping these two variables since it makes no sense to
save the children PIDs in the following testing.



> +       int nsig, i, status;
> +
> +       pid = SAFE_FORK();
> +       if (pid == 0) {
> +               setpgrp();
> +               for (i = 0; i < 5; i++) {
> +                       child_pid[i] = SAFE_FORK();
>

if we drop the arrays, that loop would be like:

                for (i = 0; i < 5; i++) {
                        if (!SAFE_FORK())
                                pause();
                }



> +                       if (!child_pid[i])
> +                               pause();
>                 }
>
> -               /*
> -                * Check to see if the process was terminated with the
> -                * expected signal.
> -                */
> -               nsig = WTERMSIG(status);
> -               if (!nsig) {
> -                       tst_resm(TFAIL, "Did not receive any signal");
> -               } else if (nsig == TEST_SIG) {
> -                       tst_resm(TPASS, "received expected signal %d",
> -                                nsig);
> -               } else {
> -                       tst_resm(TFAIL,
> -                                "expected signal %d received %d",
> -                                TEST_SIG, nsig);
> -               }
> +               TEST(kill(-getpgrp(), SIGKILL));
> +               if (TST_RET != 0)
> +                       tst_res(TFAIL | TTERRNO, "kill failed");
> +               tst_res(TINFO, "%d never received a signal", getpid());
>

This TINFO is a misleading message to people. If the process receives a
signal
but not been killed, how do you say that never received a signal?
So I suggest deleting this line.


> +               exit(0);
>         }
> -
> -       cleanup();
> -       tst_exit();
> -}
> -
> -/*
> - * do_child()
> - */
> -void do_child(void)
> -{
> -       int exno = 1;
> -
> -       sleep(299);
> -
> -       tst_resm(TINFO, "%d never received a" " signal", getpid());
> -       exit(exno);
> -}
> -
> -/*
> - * setup() - performs all ONE TIME setup for this test
> - */
> -void setup(void)
> -{
> -       /* Setup default signal handling */
> -       tst_sig(FORK, DEF_HANDLER, cleanup);
> -
> -       TEST_PAUSE;
> +
> +       SAFE_WAITPID(pid, &status, 0);
>

What about waiting for any child process here?
  SAFE_WAITPID(-1, &status, 0);



> +       nsig = WTERMSIG(status);
> +       if (nsig != SIGKILL) {
> +               tst_res(TFAIL, "wait: unexpected signal %d returned, "
> +                       "expected SIGKILL(9)", nsig);
> +               return;
> +       }
> +       tst_res(TPASS, "receive expected signal SIGKILL(9)");
>  }
>


-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200821/43a364ab/attachment-0001.htm>

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

end of thread, other threads:[~2020-08-21  6:57 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-20 10:26 [LTP] [PATCH 1/4] syscalls/kill01: Remove it Feiyu Zhu
2020-08-20 10:26 ` [LTP] [PATCH 2/4] syscalls/kill03, 04: Cleanup && Convert to new library Feiyu Zhu
2020-08-20 12:18   ` Li Wang
2020-08-21  1:18     ` Yang Xu
2020-08-21  1:56       ` Li Wang
2020-08-21  2:21         ` Yang Xu
2020-08-21  3:16           ` Li Wang
2020-08-21  3:34             ` Yang Xu
2020-08-21  3:43               ` Li Wang
2020-08-20 10:26 ` [LTP] [PATCH 3/4] syscalls/kill05:Cleanup " Feiyu Zhu
2020-08-21  5:49   ` Li Wang
2020-08-20 10:26 ` [LTP] [PATCH 4/4] syscalls/kill06:Cleanup " Feiyu Zhu
2020-08-21  6:57   ` Li Wang
2020-08-20 13:22 ` [LTP] [PATCH 1/4] syscalls/kill01: Remove it Cyril Hrubis

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.