[LTP] [PATCH v3 1/5] syscalls/clone02: Convert to new API

[LTP] [PATCH v3 1/5] syscalls/clone02: Convert to new API

[zhanglianjie]

Signed-off-by: zhanglianjie <zhanglianjie@uniontech.com>

diff --git a/testcases/kernel/syscalls/clone/clone02.c b/testcases/kernel/syscalls/clone/clone02.c
index 42eea135a..c6069d59f 100644
--- a/testcases/kernel/syscalls/clone/clone02.c
+++ b/testcases/kernel/syscalls/clone/clone02.c
@@ -1,468 +1,210 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (c) Wipro Technologies Ltd, 2002.  All Rights Reserved.
  * Copyright (c) 2012 Wanlong Gao <gaowanlong@cn.fujitsu.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
  */
-/*
- *	 TEST1
- *	 -----
- *		Call clone() with all resources shared.
- *
- *		CHILD:
- *			modify the shared resources
- *			return 1 on success
- *		PARENT:
- *			wait for child to finish
- *			verify that the shared resourses are modified
- *			return 1 on success
- *		If parent & child returns successfully
- *			test passed
- *		else
- *			test failed
+
+/*\
+ * [Description]
+ *   TEST1
+ * - call clone() with all resources shared.
+ * - child modify the shared resources.
+ * - parent wait for child to finish.
+ * - verify that the shared resourses are modified.
+ * - if the parent shared resource is modified, then pass.
  *
  *	 TEST2
- *	 -----
- *		Call clone() with no resources shared.
- *
- *		CHILD:
- *			modify the resources in child's address space
- *			return 1 on success
- *		PARENT:
- *			wait for child to finish
- *			verify that the parent's resourses are not modified
- *			return 1 on success
- *		If parent & child returns successfully
- *			test passed
- *		else
- *			test failed
+ * - call clone() with no resources shared.
+ * - modify the resources in child's address space.
+ * - parent wait for child to finish.
+ * - verify that the parent's resourses are not modified.
+ * - if the parent shared resource are modified, then pass.
  */

-#if defined UCLINUX && !__THROW
-/* workaround for libc bug */
-#define __THROW
-#endif
-
 #define _GNU_SOURCE

-#include <errno.h>
-#include <fcntl.h>
-#include <sys/wait.h>
-#include <sys/types.h>
-#include <sys/syscall.h>
 #include <sched.h>
-#include "test.h"
-#include "safe_macros.h"
+#include <stdio.h>
+#include <stdlib.h>
+#include "tst_test.h"
+#include "lapi/syscalls.h"
+#include "clone_platform.h"

+#define TESTFILE "testfile"
+#define TESTDIR "testdir"
 #define FLAG_ALL (CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|SIGCHLD)
 #define FLAG_NONE SIGCHLD
 #define PARENT_VALUE 1
 #define CHILD_VALUE 2
-#define TRUE 1
-#define FALSE 0
-
-#include "clone_platform.h"
-
-static void setup(void);
-static int test_setup(void);
-static void cleanup(void);
-static void test_cleanup(void);
-static int child_fn();
-static int parent_test1(void);
-static int parent_test2(void);
-static int test_VM(void);
-static int test_FS(void);
-static int test_FILES(void);
-static int test_SIG(void);
-static int modified_VM(void);
-static int modified_FS(void);
-static int modified_FILES(void);
-static int modified_SIG(void);
-static void sig_child_defined_handler(int);
-static void sig_default_handler();

 static int fd_parent;
-static char file_name[25];
 static int parent_variable;
-static char cwd_parent[FILENAME_MAX];
+static char *cwd_parent;
 static int parent_got_signal, child_pid;
+static void *child_stack;
+static char cwd_child[FILENAME_MAX];

-char *TCID = "clone02";
-
-struct test_case_t {
-	int flags;
-	int (*parent_fn) ();
-} test_cases[] = {
-	{
-	FLAG_ALL, parent_test1}, {
-	FLAG_NONE, parent_test2}
-};
-
-int TST_TOTAL = sizeof(test_cases) / sizeof(test_cases[0]);
-
-int main(int ac, char **av)
+static void sig_child_defined_handler(int pid LTP_ATTRIBUTE_UNUSED)
 {
+	TEST(tst_syscall(__NR_gettid));
+	if (TST_RET == child_pid)
+		tst_res(TWARN, "Child got SIGUSR2 signal");
+	else
+		parent_got_signal = 1;
+}

-	int lc;
-	void *child_stack;
-	int status, i;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	child_stack = malloc(CHILD_STACK_SIZE);
-	if (child_stack == NULL)
-		tst_brkm(TBROK, cleanup, "Cannot allocate stack for child");
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		for (i = 0; i < TST_TOTAL; ++i) {
-			if (test_setup() != 0) {
-				tst_resm(TWARN, "test_setup() failed, skipping this test case");
-				continue;
-			}
-
-			/* Test the system call */
-			TEST(ltp_clone(test_cases[i].flags, child_fn, NULL,
-				       CHILD_STACK_SIZE, child_stack));
-
-			/* check return code */
-			if (TEST_RETURN == -1) {
-				tst_resm(TFAIL | TTERRNO, "clone() failed");
-				/* Cleanup & continue with next test case */
-				test_cleanup();
-				continue;
-			}
-
-			/* Wait for child to finish */
-			if ((wait(&status)) == -1) {
-				tst_resm(TWARN | TERRNO,
-					 "wait failed; skipping testcase");
-				/* Cleanup & continue with next test case */
-				test_cleanup();
-				continue;
-			}
-
-			if (WTERMSIG(status))
-				tst_resm(TWARN, "child exitied with signal %d",
-					 WTERMSIG(status));
-
-			/*
-			 * Check the return value from child function and
-			 * parent function. If both functions returned
-			 * successfully, test passed, else failed
-			 */
-			if (WIFEXITED(status) && WEXITSTATUS(status) == 0 &&
-			    test_cases[i].parent_fn())
-				tst_resm(TPASS, "Test Passed");
-			else
-				tst_resm(TFAIL, "Test Failed");
-
-			/* Do test specific cleanup */
-			test_cleanup();
-		}
-	}
+static void modify_resource(void)
+{
+	struct sigaction new_act;

-	free(child_stack);
+	parent_variable = CHILD_VALUE;

-	cleanup();
-	tst_exit();
-}
+	close(fd_parent);

-static void setup(void)
-{
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-	TEST_PAUSE;
-	tst_tmpdir();
+	SAFE_CHDIR(cwd_child);

-	/* Get unique file name for each child process */
-	if ((sprintf(file_name, "parent_file_%ld", syscall(__NR_gettid))) <= 0)
-		tst_brkm(TBROK | TERRNO, cleanup, "sprintf() failed");
-}
+	new_act.sa_handler = sig_child_defined_handler;
+	new_act.sa_flags = SA_RESTART;
+	SAFE_SIGEMPTYSET(&new_act.sa_mask);

-static void cleanup(void)
-{
-	if (unlink(file_name) == -1)
-		tst_resm(TWARN | TERRNO, "unlink(%s) failed", file_name);
-	tst_rmdir();
+	SAFE_SIGACTION(SIGUSR2, &new_act, NULL);
+	SAFE_KILL(getppid(), SIGUSR2);
 }

-static int test_setup(void)
+static int verify_resource(void)
 {
+	char buff[20];
+	char cwd[FILENAME_MAX];

-	struct sigaction def_act;
+	int changed = 0;

-	/* Save current working directory of parent */
-	if (getcwd(cwd_parent, sizeof(cwd_parent)) == NULL) {
-		tst_resm(TWARN | TERRNO, "getcwd() failed in test_setup()");
-		return -1;
+	if (parent_variable == CHILD_VALUE) {
+		changed++;
 	}

-	/*
-	 * Set value for parent_variable in parent, which will be
-	 * changed by child in test_VM(), for testing CLONE_VM flag
-	 */
-	parent_variable = PARENT_VALUE;
-
-	/*
-	 * Open file from parent, which will be closed by
-	 * child in test_FILES(), used for testing CLONE_FILES flag
-	 */
-	fd_parent = open(file_name, O_CREAT | O_RDWR, 0777);
-	if (fd_parent == -1) {
-		tst_resm(TWARN | TERRNO, "open() failed in test_setup()");
-		return -1;
+	if (((read(fd_parent, buff, sizeof(buff))) == -1) && (errno == EBADF)) {
+		changed++;
+	} else {
+		SAFE_CLOSE(fd_parent);
 	}

-	/*
-	 * set parent_got_signal to FALSE, used for testing
-	 * CLONE_SIGHAND flag
-	 */
-	parent_got_signal = FALSE;
-
-	/* Setup signal handler for SIGUSR2 */
-	def_act.sa_handler = sig_default_handler;
-	def_act.sa_flags = SA_RESTART;
-	sigemptyset(&def_act.sa_mask);
+	SAFE_GETCWD(cwd, sizeof(cwd));
+	if (strcmp(cwd, cwd_parent)) {
+		changed++;
+	}

-	if (sigaction(SIGUSR2, &def_act, NULL) == -1) {
-		tst_resm(TWARN | TERRNO, "sigaction() failed in test_setup()");
-		return -1;
+	if (parent_got_signal) {
+		changed++;
 	}

-	return 0;
+	return changed;
 }

-static void test_cleanup(void)
+static void sig_parent_default_handler(int pid LTP_ATTRIBUTE_UNUSED)
 {

-	/* Restore parent's working directory */
-	SAFE_CHDIR(cleanup, cwd_parent);
-
 }

-static int child_fn(void)
+static int child_fn(void *arg LTP_ATTRIBUTE_UNUSED)
 {
+	TEST(tst_syscall(__NR_gettid));
+	child_pid = TST_RET;

-	/* save child pid */
-	child_pid = syscall(__NR_gettid);
-
-	if (test_VM() == 0 && test_FILES() == 0 && test_FS() == 0 &&
-	    test_SIG() == 0)
-		_exit(0);
-	_exit(1);
+	modify_resource();
+	_exit(0);
 }

 static int parent_test1(void)
 {
-
+	int ret;
 	/*
-	 * For first test case (with all flags set), all resources are
-	 * shared between parent and child. So whatever changes made by
-	 * child should get reflected in parent also. modified_*()
-	 * functions check this. All of them should return 1 for
-	 * parent_test1() to return 1
+	 * Verify that the parent process resource has changed
 	 */
+	ret = verify_resource();

-	if (modified_VM() && modified_FILES() && modified_FS() &&
-	    modified_SIG())
-		return 0;
-	return -1;
+	return ret == 4 ? 0 : -1;
 }

 static int parent_test2(void)
 {
-
 	/*
-	 * For second test case (with no resouce shared), all of the
-	 * modified_*() functions should return 0 for parent_test2()
-	 * to return 1
+	 * Verify that the parent process resource has not changed
 	 */
-	if (modified_VM() || modified_FILES() || modified_FS() ||
-	    modified_SIG())
-		return 0;
-
-	return -1;
-}
-
-/*
- * test_VM() - function to change parent_variable from child's
- *	       address space. If CLONE_VM flag is set, child shares
- *	       the memory space with parent so this will be visible
- *	       to parent also.
- */
-
-static int test_VM(void)
-{
-	parent_variable = CHILD_VALUE;
-	return 0;
-}
-
-/*
- * test_FILES() - This function closes a file descriptor opened by
- *		  parent. If CLONE_FILES flag is set, the parent and
- *		  the child process share the same file descriptor
- *		  table. so this affects the parent also
- */
-static int test_FILES(void)
-{
-	if (close(fd_parent) == -1) {
-		tst_resm(TWARN | TERRNO, "close failed in test_FILES");
-		return -1;
-	}
-	return 0;
+	return verify_resource();
 }

-/*
- * test_FS() -  This function changes the current working directory
- *		of the child process. If CLONE_FS flag is set, this
- *		will be visible to parent also.
- */
-static int test_FS(void)
-{
-	char *test_tmpdir;
-	int rval;
-
-	test_tmpdir = tst_get_tmpdir();
-	if (test_tmpdir == NULL) {
-		tst_resm(TWARN | TERRNO, "tst_get_tmpdir failed");
-		rval = -1;
-	} else if (chdir(test_tmpdir) == -1) {
-		tst_resm(TWARN | TERRNO, "chdir failed in test_FS");
-		rval = -1;
-	} else {
-		rval = 0;
-	}
-
-	free(test_tmpdir);
-	return rval;
-}
+struct tcase {
+	unsigned long flags;
+	int (*parent_fn) ();
+	char *desc;
+} tcases[] = {
+	{FLAG_ALL, parent_test1, "clone() with all resources shared"},
+	{FLAG_NONE, parent_test2, "clone() with all no resources shared"}
+};

-/*
- * test_SIG() - This function changes the signal handler for SIGUSR2
- *		signal for child. If CLONE_SIGHAND flag is set, this
- *		affects parent also.
- */
-static int test_SIG(void)
+static void verify_clone(void)
 {
+	TST_EXP_PID_SILENT(ltp_clone(tcases[tst_variant].flags, child_fn, NULL,
+				CHILD_STACK_SIZE, child_stack));

-	struct sigaction new_act;
+	if (!TST_PASS)
+		return;

-	new_act.sa_handler = sig_child_defined_handler;
-	new_act.sa_flags = SA_RESTART;
-	sigemptyset(&new_act.sa_mask);
-
-	/* Set signal handler to sig_child_defined_handler */
-	if (sigaction(SIGUSR2, &new_act, NULL) == -1) {
-		tst_resm(TWARN | TERRNO, "signal failed in test_SIG");
-		return -1;
-	}
-
-	/* Send SIGUSR2 signal to parent */
-	if (kill(getppid(), SIGUSR2) == -1) {
-		tst_resm(TWARN | TERRNO, "kill failed in test_SIG");
-		return -1;
-	}
+	tst_reap_children();

-	return 0;
+	TST_EXP_PASS(tcases[tst_variant].parent_fn(), "%s", tcases[tst_variant].desc);
 }

-/*
- * modified_VM() - This function is called by parent process to check
- *		   whether child's modification to parent_variable
- *		   is visible to parent
- */
-
-static int modified_VM(void)
-{
-
-	if (parent_variable == CHILD_VALUE)
-		/* child has modified parent_variable */
-		return 1;
-
-	return 0;
-}

-/*
- * modified_FILES() - This function checks for file descriptor table
- *		      modifications done by child
- */
-static int modified_FILES(void)
+static void cleanup(void)
 {
-	char buff[20];
+	SAFE_CHDIR(cwd_parent);
+	SAFE_UNLINK(TESTFILE);
+	SAFE_RMDIR(cwd_child);

-	if (((read(fd_parent, buff, sizeof(buff))) == -1) && (errno == EBADF))
-		/* Child has closed this file descriptor */
-		return 1;
-
-	/* close fd_parent */
-	if ((close(fd_parent)) == -1)
-		tst_resm(TWARN | TERRNO, "close() failed in modified_FILES()");
-
-	return 0;
+	free(cwd_parent);
+	free(child_stack);
 }

-/*
- * modified_FS() - This function checks parent's current working directory
- *		   to see whether its modified by child or not.
- */
-static int modified_FS(void)
+static void setup(void)
 {
-	char cwd[FILENAME_MAX];
-
-	if ((getcwd(cwd, sizeof(cwd))) == NULL)
-		tst_resm(TWARN | TERRNO, "getcwd() failed");
+	struct sigaction def_act;

-	if (!(strcmp(cwd, cwd_parent)))
-		/* cwd hasn't changed */
-		return 0;
+	/* Save current working directory of parent */
+	cwd_parent = tst_get_tmpdir();

-	return 1;
-}
+	/*
+	 * Set value for parent_variable in parent, which will be
+	 * changed by child for testing CLONE_VM flag
+	 */
+	parent_variable = PARENT_VALUE;

-/*
- * modified_SIG() - This function checks whether child has changed
- *		    parent's signal handler for signal, SIGUSR2
- */
-static int modified_SIG(void)
-{
+	/*
+	 * Open file from parent, which will be closed by
+	 * child, used for testing CLONE_FILES flag
+	 */
+	fd_parent = SAFE_OPEN(TESTFILE, O_CREAT | O_RDWR, 0777);

-	if (parent_got_signal)
-		/*
-		 * parent came through sig_child_defined_handler()
-		 * this means child has changed parent's handler
-		 */
-		return 1;
+	/*
+	 * set parent_got_signal to 0, used for testing
+	 * CLONE_SIGHAND flag
+	 */
+	parent_got_signal = 0;

-	return 0;
-}
+	def_act.sa_handler = sig_parent_default_handler;
+	def_act.sa_flags = SA_RESTART;
+	SAFE_SIGEMPTYSET(&def_act.sa_mask);
+	SAFE_SIGACTION(SIGUSR2, &def_act, NULL);

-/*
- * sig_child_defined_handler()  - Signal handler installed by child
- */
-static void sig_child_defined_handler(int pid)
-{
-	if ((syscall(__NR_gettid)) == child_pid)
-		/* Child got signal, give warning */
-		tst_resm(TWARN, "Child got SIGUSR2 signal");
-	else
-		parent_got_signal = TRUE;
+	SAFE_MKDIR(TESTDIR, 0777);
+	sprintf(cwd_child, "%s/%s", cwd_parent, TESTDIR);
+	child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
 }

-/* sig_default_handler() - Default handler for parent */
-static void sig_default_handler(void)
-{
-}
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_variants = ARRAY_SIZE(tcases),
+	.test_all = verify_clone,
+	.needs_tmpdir = 1,
+};
--
2.20.1




-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 2/5] syscalls/clone03: Convert to new API

[zhanglianjie]

Signed-off-by: zhanglianjie <zhanglianjie@uniontech.com>

diff --git a/testcases/kernel/syscalls/clone/clone03.c b/testcases/kernel/syscalls/clone/clone03.c
index f4216ead8..81d6ee649 100644
--- a/testcases/kernel/syscalls/clone/clone03.c
+++ b/testcases/kernel/syscalls/clone/clone03.c
@@ -1,147 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (c) Wipro Technologies Ltd, 2002.  All Rights Reserved.
  * Copyright (c) 2012 Wanlong Gao <gaowanlong@cn.fujitsu.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
  */
-/*
+
+/*\
+ * [Description]
  *	Check for equality of pid of child & return value of clone(2)
- *	Test:
- *	 Open a pipe.
- *	 Loop if the proper options are given.
- *	  Call clone(2) called without SIGCHLD
- *
- *	  CHILD:
- *		writes the pid to pipe
- *	  PARENT:
- *		reads child'd pid from pipe
- *		if child's pid == return value from clone(2)
- *			Test passed
- *		else
- *			test failed
  */
-
-#if defined UCLINUX && !__THROW
-/* workaround for libc bug */
-#define __THROW
-#endif
-
-#include <errno.h>
-#include <sched.h>
-#include <sys/wait.h>
-#include "test.h"
-
+#include <stdio.h>
+#include <stdlib.h>
+#include "tst_test.h"
 #include "clone_platform.h"

-static void setup(void);
-static void cleanup(void);
-static int child_fn();
-
-static int pfd[2];
+static void *child_stack;
+static char *channel;

-char *TCID = "clone03";
-int TST_TOTAL = 1;
-
-int main(int ac, char **av)
+static int child_fn(void *arg LTP_ATTRIBUTE_UNUSED)
 {
+	sprintf(channel, "%d", getpid());
+	exit(0);
+}

-	int lc;
-	void *child_stack;
-	char buff[10];
-	int child_pid, status;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	/* Allocate stack for child */
-	child_stack = malloc(CHILD_STACK_SIZE);
-	if (child_stack == NULL)
-		tst_brkm(TBROK, cleanup, "Cannot allocate stack for child");
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		if ((pipe(pfd)) == -1)
-			tst_brkm(TBROK | TERRNO, cleanup, "pipe failed");
-
-		TEST(ltp_clone(SIGCHLD, child_fn, NULL, CHILD_STACK_SIZE,
-			       child_stack));
-
-		if (TEST_RETURN == -1)
-			tst_brkm(TFAIL | TTERRNO, cleanup, "clone() failed");
-
-		/* close write end from parent */
-		if ((close(pfd[1])) == -1)
-			tst_brkm(TBROK | TERRNO, cleanup,
-				 "close(pfd[1]) failed");
-
-		/* Read pid from read end */
-		if ((read(pfd[0], buff, sizeof(buff))) == -1)
-			tst_brkm(TBROK | TERRNO, cleanup,
-				 "read from pipe failed");
+static void verify_clone(void)
+{
+	int child_pid;

-		/* Close read end from parent */
-		if ((close(pfd[0])) == -1)
-			tst_resm(TWARN | TERRNO, "close(pfd[0]) failed");
+	TST_EXP_PID_SILENT(ltp_clone(SIGCHLD, child_fn, NULL, CHILD_STACK_SIZE,
+				child_stack));

-		/* Get child's pid from pid string */
-		child_pid = atoi(buff);
+	if (!TST_PASS)
+		return;

-		if (TEST_RETURN == child_pid)
-			tst_resm(TPASS, "Test passed");
-		else
-			tst_resm(TFAIL, "Test failed");
+	tst_reap_children();

-		if ((wait(&status)) == -1)
-			tst_brkm(TBROK | TERRNO, cleanup,
-				 "wait failed, status: %d", status);
-	}
+	child_pid = atoi(channel);

-	free(child_stack);
-
-	cleanup();
-	tst_exit();
+	TST_EXP_PASS(!(TST_RET == child_pid), "pid(%d)", child_pid);
 }

 static void setup(void)
 {
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-	TEST_PAUSE;
+	child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
+	channel = SAFE_MMAP(NULL, 10, PROT_READ | PROT_WRITE,
+				MAP_SHARED | MAP_ANONYMOUS, -1, 0);
 }

 static void cleanup(void)
 {
-}
-
-static int child_fn(void)
-{
-	char pid[10];
-
-	/* Close read end from child */
-	if ((close(pfd[0])) == -1)
-		perror("close(pfd[0]) failed");
-
-	sprintf(pid, "%d", getpid());
-
-	/* Write pid string to pipe */
-	if ((write(pfd[1], pid, sizeof(pid))) == -1)
-		perror("write to pipe failed");
-
-	/* Close write end of pipe from child */
-	if ((close(pfd[1])) == -1)
-		perror("close(pfd[1]) failed");
+	free(child_stack);

-	exit(1);
+	if (channel)
+		SAFE_MUNMAP(channel, 10);
 }
+
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = verify_clone,
+	.cleanup = cleanup,
+};
--
2.20.1




-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 3/5] syscalls/clone05: Convert to new API

[zhanglianjie]

Signed-off-by: zhanglianjie <zhanglianjie@uniontech.com>

diff --git a/testcases/kernel/syscalls/clone/clone05.c b/testcases/kernel/syscalls/clone/clone05.c
index 494b772dd..79e08b925 100644
--- a/testcases/kernel/syscalls/clone/clone05.c
+++ b/testcases/kernel/syscalls/clone/clone05.c
@@ -1,103 +1,63 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (c) Wipro Technologies Ltd, 2002.  All Rights Reserved.
  * Copyright (c) 2012 Wanlong Gao <gaowanlong@cn.fujitsu.com>
  * Copyright (c) 2012 Cyril Hrubis <chrubis@suse.cz>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
  */

-/*
+/*\
+ * [Description]
  * Call clone() with CLONE_VFORK flag set. verify that
  * execution of parent is suspended until child finishes
  */

 #define _GNU_SOURCE

-#include <errno.h>
+#include <stdlib.h>
 #include <sched.h>
-#include <sys/wait.h>
-#include "test.h"
+#include "tst_test.h"
 #include "clone_platform.h"

-char *TCID = "clone05";
-int TST_TOTAL = 1;
-
-static void setup(void);
-static void cleanup(void);
-static int child_fn(void *);
-
-static int child_exited = 0;
+static int child_exited;
+static void *child_stack;

-int main(int ac, char **av)
+static int child_fn(void *unused LTP_ATTRIBUTE_UNUSED)
 {
+	int i;

-	int lc, status;
-	void *child_stack;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	child_stack = malloc(CHILD_STACK_SIZE);
-	if (child_stack == NULL)
-		tst_brkm(TBROK, cleanup, "Cannot allocate stack for child");
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		TEST(ltp_clone(CLONE_VM | CLONE_VFORK | SIGCHLD, child_fn, NULL,
-		               CHILD_STACK_SIZE, child_stack));
-
-		if ((TEST_RETURN != -1) && (child_exited))
-			tst_resm(TPASS, "Test Passed");
-		else
-			tst_resm(TFAIL, "Test Failed");
+	for (i = 0; i < 100; i++) {
+		sched_yield();
+		usleep(1000);
+	}

-		if ((wait(&status)) == -1)
-			tst_brkm(TBROK | TERRNO, cleanup,
-				 "wait failed, status: %d", status);
+	child_exited = 1;
+	_exit(0);
+}

-		child_exited = 0;
-	}
+static void verify_clone(void)
+{
+	TST_EXP_PID_SILENT(ltp_clone(CLONE_VM | CLONE_VFORK | SIGCHLD, child_fn, NULL,
+					CHILD_STACK_SIZE, child_stack), "clone with vfork");

-	free(child_stack);
+	if (!TST_PASS)
+		return;

-	cleanup();
-	tst_exit();
+	TST_EXP_VAL(child_exited, 1);
 }

 static void setup(void)
 {
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
+	child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
+	child_exited = 0;
 }

 static void cleanup(void)
 {
+	free(child_stack);
 }

-static int child_fn(void *unused __attribute__((unused)))
-{
-	int i;
-
-	/* give the kernel scheduler chance to run the parent */
-	for (i = 0; i < 100; i++) {
-		sched_yield();
-		usleep(1000);
-	}
-
-	child_exited = 1;
-	_exit(1);
-}
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = verify_clone,
+	.cleanup = cleanup,
+};
--
2.20.1




-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 4/5] syscalls/clone06: Convert to new API

[zhanglianjie]

Signed-off-by: zhanglianjie <zhanglianjie@uniontech.com>

diff --git a/testcases/kernel/syscalls/clone/clone06.c b/testcases/kernel/syscalls/clone/clone06.c
index 99e7f3864..868fba4a5 100644
--- a/testcases/kernel/syscalls/clone/clone06.c
+++ b/testcases/kernel/syscalls/clone/clone06.c
@@ -1,140 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (c) Wipro Technologies Ltd, 2002.  All Rights Reserved.
  * Copyright (c) 2012 Wanlong Gao <gaowanlong@cn.fujitsu.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
  */

-/*
- *	Test to verify inheritance of environment variables by child.
+/*\
+ * [Description]
+ * Test to verify inheritance of environment variables by child.
  */

-#if defined UCLINUX && !__THROW
-/* workaround for libc bug */
-#define __THROW
-#endif
-
-#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
 #include <sched.h>
-#include <sys/wait.h>
-#include <fcntl.h>
-#include "test.h"
+#include "tst_test.h"
 #include "clone_platform.h"

 #define MAX_LINE_LENGTH 256

-static void setup(void);
-static void cleanup(void);
-static int child_environ();
-
-static int pfd[2];
-
-char *TCID = "clone06";
-int TST_TOTAL = 1;
+static char *buff;
+static void *child_stack;

-int main(int ac, char **av)
+static int child_environ(void *arg LTP_ATTRIBUTE_UNUSED)
 {
+	sprintf(buff, "%s", getenv("TERM") ? : "");
+	exit(0);
+}

-	int lc, status;
-	void *child_stack;
+static void verify_clone(void)
+{
 	char *parent_env;
-	char buff[MAX_LINE_LENGTH];
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	child_stack = malloc(CHILD_STACK_SIZE);
-	if (child_stack == NULL)
-		tst_brkm(TBROK, cleanup, "Cannot allocate stack for child");
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		if ((pipe(pfd)) == -1)
-			tst_brkm(TBROK | TERRNO, cleanup, "pipe() failed");
-
-		TEST(ltp_clone(SIGCHLD, child_environ, NULL, CHILD_STACK_SIZE,
-			       child_stack));
-
-		if (TEST_RETURN == -1)
-			tst_brkm(TFAIL | TTERRNO, cleanup, "clone failed");
-
-		/* close write end from parent */
-		if ((close(pfd[1])) == -1)
-			tst_resm(TWARN | TERRNO, "close(pfd[1]) failed");
-
-		/* Read env var from read end */
-		if ((read(pfd[0], buff, sizeof(buff))) == -1)
-			tst_brkm(TBROK | TERRNO, cleanup,
-				 "read from pipe failed");

-		/* Close read end from parent */
-		if ((close(pfd[0])) == -1)
-			tst_resm(TWARN | TERRNO, "close(pfd[0]) failed");
+	TST_EXP_PID_SILENT(ltp_clone(SIGCHLD, child_environ, NULL, CHILD_STACK_SIZE,
+				child_stack));

-		parent_env = getenv("TERM") ? : "";
+	if (!TST_PASS)
+		return;

-		if ((strcmp(buff, parent_env)) == 0)
-			tst_resm(TPASS, "Test Passed");
-		else
-			tst_resm(TFAIL, "Test Failed");
+	tst_reap_children();

-		if ((wait(&status)) == -1)
-			tst_brkm(TBROK | TERRNO, cleanup,
-				 "wait failed, status: %d", status);
-	}
-
-	free(child_stack);
-
-	cleanup();
-	tst_exit();
+	parent_env = getenv("TERM") ? : "";
+	TST_EXP_VAL(strcmp(buff, parent_env), 0,
+				"verify environment variables by child");
 }

 static void setup(void)
 {
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-	TEST_PAUSE;
+	child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
+	buff = SAFE_MMAP(NULL, MAX_LINE_LENGTH, PROT_READ | PROT_WRITE,
+			MAP_SHARED | MAP_ANONYMOUS, -1, 0);
 }

 static void cleanup(void)
 {
-}
-
-/*
- *	Function executed by child.
- *	Gets the value for environment variable,TERM &
- *	writes it to  a pipe.
- */
-static int child_environ(void)
-{
-
-	char var[MAX_LINE_LENGTH];
-
-	/* Close read end from child */
-	if ((close(pfd[0])) == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "close(pfd[0]) failed");
-
-	if ((sprintf(var, "%s", getenv("TERM") ? : "")) < 0)
-		tst_resm(TWARN | TERRNO, "sprintf() failed");
-
-	if ((write(pfd[1], var, MAX_LINE_LENGTH)) == -1)
-		tst_resm(TWARN | TERRNO, "write to pipe failed");
-
-	/* Close write end of pipe from child */
-	if ((close(pfd[1])) == -1)
-		tst_resm(TWARN | TERRNO, "close(pfd[1]) failed");
+	free(child_stack);

-	exit(0);
+	if (buff)
+		SAFE_MUNMAP(buff, MAX_LINE_LENGTH);
 }
+
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = verify_clone,
+	.cleanup = cleanup,
+};
--
2.20.1




-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 5/5] syscalls/clone07: Convert to new API

[zhanglianjie]

Signed-off-by: zhanglianjie <zhanglianjie@uniontech.com>

diff --git a/testcases/kernel/syscalls/clone/clone07.c b/testcases/kernel/syscalls/clone/clone07.c
index 4b2e04ee7..304810c40 100644
--- a/testcases/kernel/syscalls/clone/clone07.c
+++ b/testcases/kernel/syscalls/clone/clone07.c
@@ -1,86 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (c) International Business Machines  Corp., 2003.
  * Copyright (c) 2012 Wanlong Gao <gaowanlong@cn.fujitsu.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- */
-/*
- *	This is a test for a glibc bug for the clone(2) system call.
  */

-#if defined UCLINUX && !__THROW
-/* workaround for libc bug */
-#define __THROW
-#endif
+/*\
+ * [Description]
+ * This is a test for a glibc bug for the clone(2) system call.
+ */

-#include <errno.h>
 #include <sched.h>
-#include <sys/wait.h>
-#include "test.h"
+#include <stdio.h>
+#include <stdlib.h>
+#include "tst_test.h"
+#include "lapi/syscalls.h"
 #include "clone_platform.h"

-#define TRUE 1
-#define FALSE 0
-
-static void setup();
-static int do_child();
-
-char *TCID = "clone07";
-int TST_TOTAL = 1;
-
-static void sigsegv_handler(int);
-static void sigusr2_handler(int);
 static int child_pid;
-static int fail = FALSE;
+static int fail;
+static void *child_stack;

-int main(int ac, char **av)
+static int do_child(void *arg LTP_ATTRIBUTE_UNUSED)
 {
+	return 0;
+}

-	int lc, status;
-	void *child_stack;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
+static void sigsegv_handler(int sig LTP_ATTRIBUTE_UNUSED)
+{
+	if (child_pid == 0) {
+		kill(getppid(), SIGUSR2);
+		_exit(42);
+	}
+}

-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-		child_stack = malloc(CHILD_STACK_SIZE);
-		if (child_stack == NULL)
-			tst_brkm(TBROK, NULL,
-				 "Cannot allocate stack for child");
+static void sigusr2_handler(int sig LTP_ATTRIBUTE_UNUSED)
+{
+	if (child_pid != 0)
+		fail = 1;
+}

-		child_pid = ltp_clone(SIGCHLD, do_child, NULL,
-				      CHILD_STACK_SIZE, child_stack);
+static void verify_clone(void)
+{
+	TST_EXP_PID_SILENT(ltp_clone(SIGCHLD, do_child, NULL, CHILD_STACK_SIZE,
+				child_stack));

-		if (child_pid < 0)
-			tst_brkm(TBROK | TERRNO, NULL, "clone failed");
+	child_pid = TST_RET;

-		if ((wait(&status)) == -1)
-			tst_brkm(TBROK | TERRNO, NULL,
-				 "wait failed, status: %d", status);
+	if (!TST_PASS)
+		return;

-		free(child_stack);
-	}
+	tst_reap_children();

-	if (fail == FALSE)
-		tst_resm(TPASS,
-			 "Use of return() in child did not cause SIGSEGV");
-	else
-		tst_resm(TFAIL, "Use of return() in child caused SIGSEGV");
+	TST_EXP_VAL(fail, 0, "Use of return() in child did not cause SIGSEGV");

-	tst_exit();
 }

 static void setup(void)
@@ -88,41 +60,26 @@ static void setup(void)
 	struct sigaction def_act;
 	struct sigaction act;

-	TEST_PAUSE;
-
 	act.sa_handler = sigsegv_handler;
 	act.sa_flags = SA_RESTART;
-	sigemptyset(&act.sa_mask);
-	if ((sigaction(SIGSEGV, &act, NULL)) == -1)
-		tst_resm(TWARN | TERRNO,
-			 "sigaction() for SIGSEGV failed in test_setup()");
+	SAFE_SIGEMPTYSET(&act.sa_mask);
+	SAFE_SIGACTION(SIGSEGV, &act, NULL);

-	/* Setup signal handler for SIGUSR2 */
 	def_act.sa_handler = sigusr2_handler;
 	def_act.sa_flags = SA_RESTART | SA_RESETHAND;
-	sigemptyset(&def_act.sa_mask);
-
-	if ((sigaction(SIGUSR2, &def_act, NULL)) == -1)
-		tst_resm(TWARN | TERRNO,
-			 "sigaction() for SIGUSR2 failed in test_setup()");
-}
+	SAFE_SIGEMPTYSET(&def_act.sa_mask);
+	SAFE_SIGACTION(SIGUSR2, &def_act, NULL);

-static int do_child(void)
-{
-	return 0;
+	child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
 }

-static void sigsegv_handler(int sig)
+static void cleanup(void)
 {
-	if (child_pid == 0) {
-		kill(getppid(), SIGUSR2);
-		_exit(42);
-	}
+	free(child_stack);
 }

-/* sig_default_handler() - Default handler for parent */
-static void sigusr2_handler(int sig)
-{
-	if (child_pid != 0)
-		fail = TRUE;
-}
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = verify_clone,
+	.cleanup = cleanup,
+};
--
2.20.1




-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 2/5] syscalls/clone03: Convert to new API

[Richard Palethorpe]

Hello,

This is a big improvement. However there are some things from the old
test which can be improved.

> +static void verify_clone(void)
> +{
> +	int child_pid;
>
> -		/* Close read end from parent */
> -		if ((close(pfd[0])) == -1)
> -			tst_resm(TWARN | TERRNO, "close(pfd[0]) failed");
> +	TST_EXP_PID_SILENT(ltp_clone(SIGCHLD, child_fn, NULL, CHILD_STACK_SIZE,
> +				child_stack));

tst_clone is the new API.

>
> -		/* Get child's pid from pid string */
> -		child_pid = atoi(buff);
> +	if (!TST_PASS)
> +		return;
>
> -		if (TEST_RETURN == child_pid)
> -			tst_resm(TPASS, "Test passed");
> -		else
> -			tst_resm(TFAIL, "Test failed");
> +	tst_reap_children();
>
> -		if ((wait(&status)) == -1)
> -			tst_brkm(TBROK | TERRNO, cleanup,
> -				 "wait failed, status: %d", status);
> -	}
> +	child_pid = atoi(channel);

atoi is deprecated (see the man page).

>
> -	free(child_stack);
> -
> -	cleanup();
> -	tst_exit();
> +	TST_EXP_PASS(!(TST_RET == child_pid), "pid(%d)", child_pid);
>  }
>
>  static void setup(void)
>  {
> -	tst_sig(FORK, DEF_HANDLER, cleanup);
> -	TEST_PAUSE;
> +	child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
> +	channel = SAFE_MMAP(NULL, 10, PROT_READ | PROT_WRITE,
> +				MAP_SHARED | MAP_ANONYMOUS, -1, 0);

You could mmap a region needed for pid_t and just read and write it like
a normal variable.


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 2/5] syscalls/clone03: Convert to new API

[Cyril Hrubis]

Hi!
> > +static void verify_clone(void)
> > +{
> > +	int child_pid;
> >
> > -		/* Close read end from parent */
> > -		if ((close(pfd[0])) == -1)
> > -			tst_resm(TWARN | TERRNO, "close(pfd[0]) failed");
> > +	TST_EXP_PID_SILENT(ltp_clone(SIGCHLD, child_fn, NULL, CHILD_STACK_SIZE,
> > +				child_stack));
> 
> tst_clone is the new API.

Actually we can't use it as it is becuase:

- these tests are tests fof clone() not for clone3() so we really have
  to use the old syscall

- even if we added flag to tst_clone to force the older syscall we would
  have to add size parameter to the structure since we need to test the
  case where we pass the stack explicitly as well

I guess that it would be better to do this eventually but I do not want
to block the cleanup because of this. We can fix this later on.

> >
> > -		/* Get child's pid from pid string */
> > -		child_pid = atoi(buff);
> > +	if (!TST_PASS)
> > +		return;
> >
> > -		if (TEST_RETURN == child_pid)
> > -			tst_resm(TPASS, "Test passed");
> > -		else
> > -			tst_resm(TFAIL, "Test failed");
> > +	tst_reap_children();
> >
> > -		if ((wait(&status)) == -1)
> > -			tst_brkm(TBROK | TERRNO, cleanup,
> > -				 "wait failed, status: %d", status);
> > -	}
> > +	child_pid = atoi(channel);
> 
> atoi is deprecated (see the man page).
> 
> >
> > -	free(child_stack);
> > -
> > -	cleanup();
> > -	tst_exit();
> > +	TST_EXP_PASS(!(TST_RET == child_pid), "pid(%d)", child_pid);
> >  }
> >
> >  static void setup(void)
> >  {
> > -	tst_sig(FORK, DEF_HANDLER, cleanup);
> > -	TEST_PAUSE;
> > +	child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
> > +	channel = SAFE_MMAP(NULL, 10, PROT_READ | PROT_WRITE,
> > +				MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> 
> You could mmap a region needed for pid_t and just read and write it like
> a normal variable.

That was my point when I suggested mmap()

it should really be:

static int *child_pid;


...
	child_pid = SAFE_MMAP(NULL, sizeof(*child_pid), ....);
...

Then we can just do *child_pid = foo and if (*child_pid == bar) in the
code.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 1/5] syscalls/clone02: Convert to new API

[Cyril Hrubis]

Hi!
> -/*
> - * test_SIG() - This function changes the signal handler for SIGUSR2
> - *		signal for child. If CLONE_SIGHAND flag is set, this
> - *		affects parent also.
> - */
> -static int test_SIG(void)
> +static void verify_clone(void)
>  {
> +	TST_EXP_PID_SILENT(ltp_clone(tcases[tst_variant].flags, child_fn, NULL,
> +				CHILD_STACK_SIZE, child_stack));
> 
> -	struct sigaction new_act;
> +	if (!TST_PASS)
> +		return;
> 
> -	new_act.sa_handler = sig_child_defined_handler;
> -	new_act.sa_flags = SA_RESTART;
> -	sigemptyset(&new_act.sa_mask);
> -
> -	/* Set signal handler to sig_child_defined_handler */
> -	if (sigaction(SIGUSR2, &new_act, NULL) == -1) {
> -		tst_resm(TWARN | TERRNO, "signal failed in test_SIG");
> -		return -1;
> -	}
> -
> -	/* Send SIGUSR2 signal to parent */
> -	if (kill(getppid(), SIGUSR2) == -1) {
> -		tst_resm(TWARN | TERRNO, "kill failed in test_SIG");
> -		return -1;
> -	}
> +	tst_reap_children();
> 
> -	return 0;
> +	TST_EXP_PASS(tcases[tst_variant].parent_fn(), "%s", tcases[tst_variant].desc);

Can we, instead of this, print PASS/FAIL for each check we do, so that
if something fails the log explains what exactly has failed?

>  }
> 
> -/*
> - * modified_VM() - This function is called by parent process to check
> - *		   whether child's modification to parent_variable
> - *		   is visible to parent
> - */
> -
> -static int modified_VM(void)
> -{
> -
> -	if (parent_variable == CHILD_VALUE)
> -		/* child has modified parent_variable */
> -		return 1;
> -
> -	return 0;
> -}
> 
> -/*
> - * modified_FILES() - This function checks for file descriptor table
> - *		      modifications done by child
> - */
> -static int modified_FILES(void)
> +static void cleanup(void)
>  {
> -	char buff[20];
> +	SAFE_CHDIR(cwd_parent);
> +	SAFE_UNLINK(TESTFILE);
> +	SAFE_RMDIR(cwd_child);
> 
> -	if (((read(fd_parent, buff, sizeof(buff))) == -1) && (errno == EBADF))
> -		/* Child has closed this file descriptor */
> -		return 1;
> -
> -	/* close fd_parent */
> -	if ((close(fd_parent)) == -1)
> -		tst_resm(TWARN | TERRNO, "close() failed in modified_FILES()");
> -
> -	return 0;
> +	free(cwd_parent);
> +	free(child_stack);
>  }
> 
> -/*
> - * modified_FS() - This function checks parent's current working directory
> - *		   to see whether its modified by child or not.
> - */
> -static int modified_FS(void)
> +static void setup(void)
>  {
> -	char cwd[FILENAME_MAX];
> -
> -	if ((getcwd(cwd, sizeof(cwd))) == NULL)
> -		tst_resm(TWARN | TERRNO, "getcwd() failed");
> +	struct sigaction def_act;
> 
> -	if (!(strcmp(cwd, cwd_parent)))
> -		/* cwd hasn't changed */
> -		return 0;
> +	/* Save current working directory of parent */
> +	cwd_parent = tst_get_tmpdir();
> 
> -	return 1;
> -}
> +	/*
> +	 * Set value for parent_variable in parent, which will be
> +	 * changed by child for testing CLONE_VM flag
> +	 */
> +	parent_variable = PARENT_VALUE;
> 
> -/*
> - * modified_SIG() - This function checks whether child has changed
> - *		    parent's signal handler for signal, SIGUSR2
> - */
> -static int modified_SIG(void)
> -{
> +	/*
> +	 * Open file from parent, which will be closed by
> +	 * child, used for testing CLONE_FILES flag
> +	 */
> +	fd_parent = SAFE_OPEN(TESTFILE, O_CREAT | O_RDWR, 0777);
> 
> -	if (parent_got_signal)
> -		/*
> -		 * parent came through sig_child_defined_handler()
> -		 * this means child has changed parent's handler
> -		 */
> -		return 1;
> +	/*
> +	 * set parent_got_signal to 0, used for testing
> +	 * CLONE_SIGHAND flag
> +	 */
> +	parent_got_signal = 0;

We have to make sure we reset the $PWD, variable, got_signal flag and
open() the file before each test iteration otherwise the test will fail
on subsequent iterations with -i 2 command line parameter.

> -	return 0;
> -}
> +	def_act.sa_handler = sig_parent_default_handler;
> +	def_act.sa_flags = SA_RESTART;
> +	SAFE_SIGEMPTYSET(&def_act.sa_mask);
> +	SAFE_SIGACTION(SIGUSR2, &def_act, NULL);
> 
> -/*
> - * sig_child_defined_handler()  - Signal handler installed by child
> - */
> -static void sig_child_defined_handler(int pid)
> -{
> -	if ((syscall(__NR_gettid)) == child_pid)
> -		/* Child got signal, give warning */
> -		tst_resm(TWARN, "Child got SIGUSR2 signal");
> -	else
> -		parent_got_signal = TRUE;
> +	SAFE_MKDIR(TESTDIR, 0777);
> +	sprintf(cwd_child, "%s/%s", cwd_parent, TESTDIR);
> +	child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);

Can we use the guarded buffer instead of MALLOC in this test as well?
Just as we do in clone01.c now.

>  }
> 
> -/* sig_default_handler() - Default handler for parent */
> -static void sig_default_handler(void)
> -{
> -}
> +static struct tst_test test = {
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_variants = ARRAY_SIZE(tcases),

This should rather be .tcnt and .test = verify_clone instead of
variants.

Test variants are usually used when the whole test is exactly same but
the TEST_*() function calls different variant of the syscall instead.

> +	.test_all = verify_clone,
> +	.needs_tmpdir = 1,
> +};

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 3/5] syscalls/clone05: Convert to new API

[Cyril Hrubis]

Hi!
> +/*\
> + * [Description]
>   * Call clone() with CLONE_VFORK flag set. verify that
>   * execution of parent is suspended until child finishes
>   */
> 
>  #define _GNU_SOURCE
> 
> -#include <errno.h>
> +#include <stdlib.h>
>  #include <sched.h>
> -#include <sys/wait.h>
> -#include "test.h"
> +#include "tst_test.h"
>  #include "clone_platform.h"
> 
> -char *TCID = "clone05";
> -int TST_TOTAL = 1;
> -
> -static void setup(void);
> -static void cleanup(void);
> -static int child_fn(void *);
> -
> -static int child_exited = 0;
> +static int child_exited;
              ^
This should be volatile as well in order to avoid compiler
mis-optimizations.

> +static void *child_stack;
> 
> -int main(int ac, char **av)
> +static int child_fn(void *unused LTP_ATTRIBUTE_UNUSED)
>  {
> +	int i;
> 
> -	int lc, status;
> -	void *child_stack;
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();
> -
> -	child_stack = malloc(CHILD_STACK_SIZE);
> -	if (child_stack == NULL)
> -		tst_brkm(TBROK, cleanup, "Cannot allocate stack for child");
> -
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		tst_count = 0;
> -
> -		TEST(ltp_clone(CLONE_VM | CLONE_VFORK | SIGCHLD, child_fn, NULL,
> -		               CHILD_STACK_SIZE, child_stack));
> -
> -		if ((TEST_RETURN != -1) && (child_exited))
> -			tst_resm(TPASS, "Test Passed");
> -		else
> -			tst_resm(TFAIL, "Test Failed");
> +	for (i = 0; i < 100; i++) {
> +		sched_yield();
> +		usleep(1000);
> +	}
> 
> -		if ((wait(&status)) == -1)
> -			tst_brkm(TBROK | TERRNO, cleanup,
> -				 "wait failed, status: %d", status);
> +	child_exited = 1;
> +	_exit(0);
> +}
> 
> -		child_exited = 0;
> -	}
> +static void verify_clone(void)
> +{
> +	TST_EXP_PID_SILENT(ltp_clone(CLONE_VM | CLONE_VFORK | SIGCHLD, child_fn, NULL,
> +					CHILD_STACK_SIZE, child_stack), "clone with vfork");
> 
> -	free(child_stack);
> +	if (!TST_PASS)
> +		return;
> 
> -	cleanup();
> -	tst_exit();
> +	TST_EXP_VAL(child_exited, 1);
>  }
> 
>  static void setup(void)
>  {
> -	tst_sig(FORK, DEF_HANDLER, cleanup);
> -
> -	TEST_PAUSE;
> +	child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
> +	child_exited = 0;

Here again this has to be cleared in the test function because
oftherwise the test will fail with -i 2.

>  }
> 
>  static void cleanup(void)
>  {
> +	free(child_stack);
>  }

Please use the guarded buffer in this test as well.

> -static int child_fn(void *unused __attribute__((unused)))
> -{
> -	int i;
> -
> -	/* give the kernel scheduler chance to run the parent */
> -	for (i = 0; i < 100; i++) {
> -		sched_yield();
> -		usleep(1000);
> -	}
> -
> -	child_exited = 1;
> -	_exit(1);
> -}
> +static struct tst_test test = {
> +	.setup = setup,
> +	.test_all = verify_clone,
> +	.cleanup = cleanup,
> +};
> --
> 2.20.1
> 
> 
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 1/5] syscalls/clone02: Convert to new API

[zhanglianjie]

Hi,
I will revise it seriously, thank you.

On 2021-10-11 23:40, Cyril Hrubis wrote:
> Hi!
>> -/*
>> - * test_SIG() - This function changes the signal handler for SIGUSR2
>> - *		signal for child. If CLONE_SIGHAND flag is set, this
>> - *		affects parent also.
>> - */
>> -static int test_SIG(void)
>> +static void verify_clone(void)
>>   {
>> +	TST_EXP_PID_SILENT(ltp_clone(tcases[tst_variant].flags, child_fn, NULL,
>> +				CHILD_STACK_SIZE, child_stack));
>>
>> -	struct sigaction new_act;
>> +	if (!TST_PASS)
>> +		return;
>>
>> -	new_act.sa_handler = sig_child_defined_handler;
>> -	new_act.sa_flags = SA_RESTART;
>> -	sigemptyset(&new_act.sa_mask);
>> -
>> -	/* Set signal handler to sig_child_defined_handler */
>> -	if (sigaction(SIGUSR2, &new_act, NULL) == -1) {
>> -		tst_resm(TWARN | TERRNO, "signal failed in test_SIG");
>> -		return -1;
>> -	}
>> -
>> -	/* Send SIGUSR2 signal to parent */
>> -	if (kill(getppid(), SIGUSR2) == -1) {
>> -		tst_resm(TWARN | TERRNO, "kill failed in test_SIG");
>> -		return -1;
>> -	}
>> +	tst_reap_children();
>>
>> -	return 0;
>> +	TST_EXP_PASS(tcases[tst_variant].parent_fn(), "%s", tcases[tst_variant].desc);
> 
> Can we, instead of this, print PASS/FAIL for each check we do, so that
> if something fails the log explains what exactly has failed?
> 
>>   }
>>...
>> - */
>> -static int modified_SIG(void)
>> -{
>> +	/*
>> +	 * Open file from parent, which will be closed by
>> +	 * child, used for testing CLONE_FILES flag
>> +	 */
>> +	fd_parent = SAFE_OPEN(TESTFILE, O_CREAT | O_RDWR, 0777);
>>
>> -	if (parent_got_signal)
>> -		/*
>> -		 * parent came through sig_child_defined_handler()
>> -		 * this means child has changed parent's handler
>> -		 */
>> -		return 1;
>> +	/*
>> +	 * set parent_got_signal to 0, used for testing
>> +	 * CLONE_SIGHAND flag
>> +	 */
>> +	parent_got_signal = 0;
> 
> We have to make sure we reset the $PWD, variable, got_signal flag and
> open() the file before each test iteration otherwise the test will fail
> on subsequent iterations with -i 2 command line parameter.
> 
>> -	return 0;
>> -}
>> +	def_act.sa_handler = sig_parent_default_handler;
>> +	def_act.sa_flags = SA_RESTART;
>> +	SAFE_SIGEMPTYSET(&def_act.sa_mask);
>> +	SAFE_SIGACTION(SIGUSR2, &def_act, NULL);
>>
>> -/*
>> - * sig_child_defined_handler()  - Signal handler installed by child
>> - */
>> -static void sig_child_defined_handler(int pid)
>> -{
>> -	if ((syscall(__NR_gettid)) == child_pid)
>> -		/* Child got signal, give warning */
>> -		tst_resm(TWARN, "Child got SIGUSR2 signal");
>> -	else
>> -		parent_got_signal = TRUE;
>> +	SAFE_MKDIR(TESTDIR, 0777);
>> +	sprintf(cwd_child, "%s/%s", cwd_parent, TESTDIR);
>> +	child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
> 
> Can we use the guarded buffer instead of MALLOC in this test as well?
> Just as we do in clone01.c now.
> 
>>   }
>>
>> -/* sig_default_handler() - Default handler for parent */
>> -static void sig_default_handler(void)
>> -{
>> -}
>> +static struct tst_test test = {
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +	.test_variants = ARRAY_SIZE(tcases),
> 
> This should rather be .tcnt and .test = verify_clone instead of
> variants.
> 
> Test variants are usually used when the whole test is exactly same but
> the TEST_*() function calls different variant of the syscall instead.
> 
>> +	.test_all = verify_clone,
>> +	.needs_tmpdir = 1,
>> +};
> 

-- 
Regards,
Zhang Lianjie



-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 4/5] syscalls/clone06: Convert to new API

[Cyril Hrubis]

Hi!
> -	cleanup();
> -	tst_exit();
> +	parent_env = getenv("TERM") ? : "";

I guess that it would make more sense to create a new variable than
depend on anything that may or may not be present on the system.

What about we setenv a variable instead and use that for the test?

#define ENV_VAL "LTP test variable value"
#define ENV_ID "LTP_CLONE_TEST"

static void setup(void)
{
	int ret;

	ret = setenv(ENV_ID, ENV_VAL, 0)
	if (ret)
		tst_brk(TBROK | TERRNO, "setenv() failed");

}


> +	TST_EXP_VAL(strcmp(buff, parent_env), 0,
> +				"verify environment variables by child");

Also there is no need to propagate the value to the parent like this,
the child process can report the result (in the new library) as well, so
this really could be as simple as:

static int do_child(void *arg LTP_ATTRIBUTE_UNUSED)
{
	const char *env_val = getenv(ENV_ID);

	if (!env_val) {
		tst_res(TFAIL, "Variable " ENV_ID " not defined in child");
		return;
	}

	if (strcmp(env_val, ENV_VAL)) {
		tst_res(TFAIL, "Variable value is different");
		return;
	}

	tst_res(TPASS, ...);
}

>  }
> 
>  static void setup(void)
>  {
> -	tst_sig(FORK, DEF_HANDLER, cleanup);
> -	TEST_PAUSE;
> +	child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
> +	buff = SAFE_MMAP(NULL, MAX_LINE_LENGTH, PROT_READ | PROT_WRITE,
> +			MAP_SHARED | MAP_ANONYMOUS, -1, 0);
>  }
> 
>  static void cleanup(void)
>  {
> -}
> -
> -/*
> - *	Function executed by child.
> - *	Gets the value for environment variable,TERM &
> - *	writes it to  a pipe.
> - */
> -static int child_environ(void)
> -{
> -
> -	char var[MAX_LINE_LENGTH];
> -
> -	/* Close read end from child */
> -	if ((close(pfd[0])) == -1)
> -		tst_brkm(TBROK | TERRNO, cleanup, "close(pfd[0]) failed");
> -
> -	if ((sprintf(var, "%s", getenv("TERM") ? : "")) < 0)
> -		tst_resm(TWARN | TERRNO, "sprintf() failed");
> -
> -	if ((write(pfd[1], var, MAX_LINE_LENGTH)) == -1)
> -		tst_resm(TWARN | TERRNO, "write to pipe failed");
> -
> -	/* Close write end of pipe from child */
> -	if ((close(pfd[1])) == -1)
> -		tst_resm(TWARN | TERRNO, "close(pfd[1]) failed");
> +	free(child_stack);
> 
> -	exit(0);
> +	if (buff)
> +		SAFE_MUNMAP(buff, MAX_LINE_LENGTH);
>  }
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.test_all = verify_clone,
> +	.cleanup = cleanup,
> +};
> --
> 2.20.1
> 
> 
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 5/5] syscalls/clone07: Convert to new API

[Cyril Hrubis]

Hi!
> diff --git a/testcases/kernel/syscalls/clone/clone07.c b/testcases/kernel/syscalls/clone/clone07.c
> index 4b2e04ee7..304810c40 100644
> --- a/testcases/kernel/syscalls/clone/clone07.c
> +++ b/testcases/kernel/syscalls/clone/clone07.c
> @@ -1,86 +1,58 @@
> +// SPDX-License-Identifier: GPL-2.0-only
>  /*
>   * Copyright (c) International Business Machines  Corp., 2003.
>   * Copyright (c) 2012 Wanlong Gao <gaowanlong@cn.fujitsu.com>
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms of version 2 of the GNU General Public License as
> - * published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it would be useful, but
> - * WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> - *
> - * You should have received a copy of the GNU General Public License along
> - * with this program; if not, write the Free Software Foundation, Inc.,
> - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> - *
> - */
> -/*
> - *	This is a test for a glibc bug for the clone(2) system call.
>   */
> 
> -#if defined UCLINUX && !__THROW
> -/* workaround for libc bug */
> -#define __THROW
> -#endif
> +/*\
> + * [Description]
> + * This is a test for a glibc bug for the clone(2) system call.

We can describe this a bit better.

Apparently the bug was that using return from the do_child() function
caused SIGSEGV so this should probably be:

 [Description]

 Test for a libc bug where exitting child function by returning from it
 caused SIGSEGV.

> + */
> 
> -#include <errno.h>
>  #include <sched.h>
> -#include <sys/wait.h>
> -#include "test.h"
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include "tst_test.h"
> +#include "lapi/syscalls.h"
>  #include "clone_platform.h"
> 
> -#define TRUE 1
> -#define FALSE 0
> -
> -static void setup();
> -static int do_child();
> -
> -char *TCID = "clone07";
> -int TST_TOTAL = 1;
> -
> -static void sigsegv_handler(int);
> -static void sigusr2_handler(int);
>  static int child_pid;
> -static int fail = FALSE;
> +static int fail;
> +static void *child_stack;
> 
> -int main(int ac, char **av)
> +static int do_child(void *arg LTP_ATTRIBUTE_UNUSED)
>  {
> +	return 0;
> +}
> 
> -	int lc, status;
> -	void *child_stack;
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();
> +static void sigsegv_handler(int sig LTP_ATTRIBUTE_UNUSED)
> +{
> +	if (child_pid == 0) {
> +		kill(getppid(), SIGUSR2);
> +		_exit(42);
> +	}
> +}
> 
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		tst_count = 0;
> -		child_stack = malloc(CHILD_STACK_SIZE);
> -		if (child_stack == NULL)
> -			tst_brkm(TBROK, NULL,
> -				 "Cannot allocate stack for child");
> +static void sigusr2_handler(int sig LTP_ATTRIBUTE_UNUSED)
> +{
> +	if (child_pid != 0)
> +		fail = 1;
> +}

I do not get why we need this complicated handler setup.

Why can't we just SAFE_WAITPID() for the child, check the status and
fail the test if the status is anything else than:

WIFEXITED(status) && WEXITSTATUS(status) == 0

> -		child_pid = ltp_clone(SIGCHLD, do_child, NULL,
> -				      CHILD_STACK_SIZE, child_stack);
> +static void verify_clone(void)
> +{
> +	TST_EXP_PID_SILENT(ltp_clone(SIGCHLD, do_child, NULL, CHILD_STACK_SIZE,
> +				child_stack));
> 
> -		if (child_pid < 0)
> -			tst_brkm(TBROK | TERRNO, NULL, "clone failed");
> +	child_pid = TST_RET;
> 
> -		if ((wait(&status)) == -1)
> -			tst_brkm(TBROK | TERRNO, NULL,
> -				 "wait failed, status: %d", status);
> +	if (!TST_PASS)
> +		return;
> 
> -		free(child_stack);
> -	}
> +	tst_reap_children();
> 
> -	if (fail == FALSE)
> -		tst_resm(TPASS,
> -			 "Use of return() in child did not cause SIGSEGV");
> -	else
> -		tst_resm(TFAIL, "Use of return() in child caused SIGSEGV");
> +	TST_EXP_VAL(fail, 0, "Use of return() in child did not cause SIGSEGV");
> 
> -	tst_exit();
>  }
> 
>  static void setup(void)
> @@ -88,41 +60,26 @@ static void setup(void)
>  	struct sigaction def_act;
>  	struct sigaction act;
> 
> -	TEST_PAUSE;
> -
>  	act.sa_handler = sigsegv_handler;
>  	act.sa_flags = SA_RESTART;
> -	sigemptyset(&act.sa_mask);
> -	if ((sigaction(SIGSEGV, &act, NULL)) == -1)
> -		tst_resm(TWARN | TERRNO,
> -			 "sigaction() for SIGSEGV failed in test_setup()");
> +	SAFE_SIGEMPTYSET(&act.sa_mask);
> +	SAFE_SIGACTION(SIGSEGV, &act, NULL);
> 
> -	/* Setup signal handler for SIGUSR2 */
>  	def_act.sa_handler = sigusr2_handler;
>  	def_act.sa_flags = SA_RESTART | SA_RESETHAND;
> -	sigemptyset(&def_act.sa_mask);
> -
> -	if ((sigaction(SIGUSR2, &def_act, NULL)) == -1)
> -		tst_resm(TWARN | TERRNO,
> -			 "sigaction() for SIGUSR2 failed in test_setup()");
> -}
> +	SAFE_SIGEMPTYSET(&def_act.sa_mask);
> +	SAFE_SIGACTION(SIGUSR2, &def_act, NULL);
> 
> -static int do_child(void)
> -{
> -	return 0;
> +	child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
>  }
> 
> -static void sigsegv_handler(int sig)
> +static void cleanup(void)
>  {
> -	if (child_pid == 0) {
> -		kill(getppid(), SIGUSR2);
> -		_exit(42);
> -	}
> +	free(child_stack);
>  }
> 
> -/* sig_default_handler() - Default handler for parent */
> -static void sigusr2_handler(int sig)
> -{
> -	if (child_pid != 0)
> -		fail = TRUE;
> -}
> +static struct tst_test test = {
> +	.setup = setup,
> +	.test_all = verify_clone,
> +	.cleanup = cleanup,
> +};
> --
> 2.20.1
> 
> 
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 4/5] syscalls/clone06: Convert to new API

[zhanglianjie]

Hi,
This is a good idea, just do it, thank you.

On 2021-10-12 17:21, Cyril Hrubis wrote:
> Hi!
>> -	cleanup();
>> -	tst_exit();
>> +	parent_env = getenv("TERM") ? : "";
> 
> I guess that it would make more sense to create a new variable than
> depend on anything that may or may not be present on the system.
> 
> What about we setenv a variable instead and use that for the test?
> 
> #define ENV_VAL "LTP test variable value"
> #define ENV_ID "LTP_CLONE_TEST"
> 
> static void setup(void)
> {
> 	int ret;
> 
> 	ret = setenv(ENV_ID, ENV_VAL, 0)
> 	if (ret)
> 		tst_brk(TBROK | TERRNO, "setenv() failed");
> 
> }
> 
> 
>> +	TST_EXP_VAL(strcmp(buff, parent_env), 0,
>> +				"verify environment variables by child");
> 
> Also there is no need to propagate the value to the parent like this,
> the child process can report the result (in the new library) as well, so
> this really could be as simple as:
> 
> static int do_child(void *arg LTP_ATTRIBUTE_UNUSED)
> {
> 	const char *env_val = getenv(ENV_ID);
> 
> 	if (!env_val) {
> 		tst_res(TFAIL, "Variable " ENV_ID " not defined in child");
> 		return;
> 	}
> 
> 	if (strcmp(env_val, ENV_VAL)) {
> 		tst_res(TFAIL, "Variable value is different");
> 		return;
> 	}
> 
> 	tst_res(TPASS, ...);
> }
> 
>>   }
>>
>>   static void setup(void)
>>   {
>> -	tst_sig(FORK, DEF_HANDLER, cleanup);
>> -	TEST_PAUSE;
>> +	child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
>> +	buff = SAFE_MMAP(NULL, MAX_LINE_LENGTH, PROT_READ | PROT_WRITE,
>> +			MAP_SHARED | MAP_ANONYMOUS, -1, 0);
>>   }
>>
>>   static void cleanup(void)
>>   {
>> -}
>> -
>> -/*
>> - *	Function executed by child.
>> - *	Gets the value for environment variable,TERM &
>> - *	writes it to  a pipe.
>> - */
>> -static int child_environ(void)
>> -{
>> -
>> -	char var[MAX_LINE_LENGTH];
>> -
>> -	/* Close read end from child */
>> -	if ((close(pfd[0])) == -1)
>> -		tst_brkm(TBROK | TERRNO, cleanup, "close(pfd[0]) failed");
>> -
>> -	if ((sprintf(var, "%s", getenv("TERM") ? : "")) < 0)
>> -		tst_resm(TWARN | TERRNO, "sprintf() failed");
>> -
>> -	if ((write(pfd[1], var, MAX_LINE_LENGTH)) == -1)
>> -		tst_resm(TWARN | TERRNO, "write to pipe failed");
>> -
>> -	/* Close write end of pipe from child */
>> -	if ((close(pfd[1])) == -1)
>> -		tst_resm(TWARN | TERRNO, "close(pfd[1]) failed");
>> +	free(child_stack);
>>
>> -	exit(0);
>> +	if (buff)
>> +		SAFE_MUNMAP(buff, MAX_LINE_LENGTH);
>>   }
>> +
>> +static struct tst_test test = {
>> +	.setup = setup,
>> +	.test_all = verify_clone,
>> +	.cleanup = cleanup,
>> +};
>> --
>> 2.20.1
>>
>>
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp
> 

-- 
Regards,
Zhang Lianjie



-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 5/5] syscalls/clone07: Convert to new API

[zhanglianjie]

Hi,

On 2021-10-12 17:37, Cyril Hrubis wrote:
> Hi!
>> diff --git a/testcases/kernel/syscalls/clone/clone07.c b/testcases/kernel/syscalls/clone/clone07.c
>> index 4b2e04ee7..304810c40 100644
>> --- a/testcases/kernel/syscalls/clone/clone07.c
>> +++ b/testcases/kernel/syscalls/clone/clone07.c
>> @@ -1,86 +1,58 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>>   /*
>>    * Copyright (c) International Business Machines  Corp., 2003.
>>    * Copyright (c) 2012 Wanlong Gao <gaowanlong@cn.fujitsu.com>
>> - *
>> - * This program is free software; you can redistribute it and/or modify it
>> - * under the terms of version 2 of the GNU General Public License as
>> - * published by the Free Software Foundation.
>> - *
>> - * This program is distributed in the hope that it would be useful, but
>> - * WITHOUT ANY WARRANTY; without even the implied warranty of
>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>> - *
>> - * You should have received a copy of the GNU General Public License along
>> - * with this program; if not, write the Free Software Foundation, Inc.,
>> - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>> - *
>> - */
>> -/*
>> - *	This is a test for a glibc bug for the clone(2) system call.
>>    */
>>
>> -#if defined UCLINUX && !__THROW
>> -/* workaround for libc bug */
>> -#define __THROW
>> -#endif
>> +/*\
>> + * [Description]
>> + * This is a test for a glibc bug for the clone(2) system call.
> 
> We can describe this a bit better.
> 
> Apparently the bug was that using return from the do_child() function
> caused SIGSEGV so this should probably be:
> 
>   [Description]
> 
>   Test for a libc bug where exitting child function by returning from it
>   caused SIGSEGV.
> 
>> + */
>>
>> -#include <errno.h>
>>   #include <sched.h>
>> -#include <sys/wait.h>
>> -#include "test.h"
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include "tst_test.h"
>> +#include "lapi/syscalls.h"
>>   #include "clone_platform.h"
>>
>> -#define TRUE 1
>> -#define FALSE 0
>> -
>> -static void setup();
>> -static int do_child();
>> -
>> -char *TCID = "clone07";
>> -int TST_TOTAL = 1;
>> -
>> -static void sigsegv_handler(int);
>> -static void sigusr2_handler(int);
>>   static int child_pid;
>> -static int fail = FALSE;
>> +static int fail;
>> +static void *child_stack;
>>
>> -int main(int ac, char **av)
>> +static int do_child(void *arg LTP_ATTRIBUTE_UNUSED)
>>   {
>> +	return 0;
>> +}
>>
>> -	int lc, status;
>> -	void *child_stack;
>> -
>> -	tst_parse_opts(ac, av, NULL, NULL);
>> -
>> -	setup();
>> +static void sigsegv_handler(int sig LTP_ATTRIBUTE_UNUSED)
>> +{
>> +	if (child_pid == 0) {
>> +		kill(getppid(), SIGUSR2);
>> +		_exit(42);
>> +	}
>> +}
>>
>> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
>> -		tst_count = 0;
>> -		child_stack = malloc(CHILD_STACK_SIZE);
>> -		if (child_stack == NULL)
>> -			tst_brkm(TBROK, NULL,
>> -				 "Cannot allocate stack for child");
>> +static void sigusr2_handler(int sig LTP_ATTRIBUTE_UNUSED)
>> +{
>> +	if (child_pid != 0)
>> +		fail = 1;
>> +}
> 
> I do not get why we need this complicated handler setup.
> 
> Why can't we just SAFE_WAITPID() for the child, check the status and
> fail the test if the status is anything else than:
> 
> WIFEXITED(status) && WEXITSTATUS(status) == 0
> 
The processing here is to accurately determine whether a segment fault 
signal has occurred in the child process. Of course, can also use the 
method you provide, but you can only judge that the child process exits 
abnormally.

>> -		child_pid = ltp_clone(SIGCHLD, do_child, NULL,
>> -				      CHILD_STACK_SIZE, child_stack);
>> +static void verify_clone(void)
>> +{
>> +	TST_EXP_PID_SILENT(ltp_clone(SIGCHLD, do_child, NULL, CHILD_STACK_SIZE,
>> +				child_stack));
>>
>> -		if (child_pid < 0)
>> -			tst_brkm(TBROK | TERRNO, NULL, "clone failed");
>> +	child_pid = TST_RET;
>>
>> -		if ((wait(&status)) == -1)
>> -			tst_brkm(TBROK | TERRNO, NULL,
>> -				 "wait failed, status: %d", status);
>> +	if (!TST_PASS)
>> +		return;
>>
>> -		free(child_stack);
>> -	}
>> +	tst_reap_children();
>>
>> -	if (fail == FALSE)
>> -		tst_resm(TPASS,
>> -			 "Use of return() in child did not cause SIGSEGV");
>> -	else
>> -		tst_resm(TFAIL, "Use of return() in child caused SIGSEGV");
>> +	TST_EXP_VAL(fail, 0, "Use of return() in child did not cause SIGSEGV");
>>
>> -	tst_exit();
>>   }
>>
>>   static void setup(void)
>> @@ -88,41 +60,26 @@ static void setup(void)
>>   	struct sigaction def_act;
>>   	struct sigaction act;
>>
>> -	TEST_PAUSE;
>> -
>>   	act.sa_handler = sigsegv_handler;
>>   	act.sa_flags = SA_RESTART;
>> -	sigemptyset(&act.sa_mask);
>> -	if ((sigaction(SIGSEGV, &act, NULL)) == -1)
>> -		tst_resm(TWARN | TERRNO,
>> -			 "sigaction() for SIGSEGV failed in test_setup()");
>> +	SAFE_SIGEMPTYSET(&act.sa_mask);
>> +	SAFE_SIGACTION(SIGSEGV, &act, NULL);
>>
>> -	/* Setup signal handler for SIGUSR2 */
>>   	def_act.sa_handler = sigusr2_handler;
>>   	def_act.sa_flags = SA_RESTART | SA_RESETHAND;
>> -	sigemptyset(&def_act.sa_mask);
>> -
>> -	if ((sigaction(SIGUSR2, &def_act, NULL)) == -1)
>> -		tst_resm(TWARN | TERRNO,
>> -			 "sigaction() for SIGUSR2 failed in test_setup()");
>> -}
>> +	SAFE_SIGEMPTYSET(&def_act.sa_mask);
>> +	SAFE_SIGACTION(SIGUSR2, &def_act, NULL);
>>
>> -static int do_child(void)
>> -{
>> -	return 0;
>> +	child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
>>   }
>>
>> -static void sigsegv_handler(int sig)
>> +static void cleanup(void)
>>   {
>> -	if (child_pid == 0) {
>> -		kill(getppid(), SIGUSR2);
>> -		_exit(42);
>> -	}
>> +	free(child_stack);
>>   }
>>
>> -/* sig_default_handler() - Default handler for parent */
>> -static void sigusr2_handler(int sig)
>> -{
>> -	if (child_pid != 0)
>> -		fail = TRUE;
>> -}
>> +static struct tst_test test = {
>> +	.setup = setup,
>> +	.test_all = verify_clone,
>> +	.cleanup = cleanup,
>> +};
>> --
>> 2.20.1
>>
>>
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp
> 

-- 
Regards,
Zhang Lianjie



-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 5/5] syscalls/clone07: Convert to new API

[Cyril Hrubis]

Hi!
> > I do not get why we need this complicated handler setup.
> > 
> > Why can't we just SAFE_WAITPID() for the child, check the status and
> > fail the test if the status is anything else than:
> > 
> > WIFEXITED(status) && WEXITSTATUS(status) == 0
> > 
> The processing here is to accurately determine whether a segment fault 
> signal has occurred in the child process. Of course, can also use the 
> method you provide, but you can only judge that the child process exits 
> abnormally.

Well I do not think that the SEGFAULT needs to have special treatement
here, as long as the child does anything else than exit with 0 it's a
bug, that's why I think that we should fail the test in all other cases.

I guess that we can add special check for the regression with
WIFSIGNALED() && WTERMSIG() == SIGSEGV and produce slightly different
TFAIL message, but we should really fail all the other cases as well.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp