| Message ID | 20191001070111.31369-1-jcronenberg@suse.de |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Convert ptrace01.c to new library | expand |
> -----Original Message----- > From: Jorik Cronenberg on Monday, September 30, 2019 9:01 PM > To: ltp@lists.linux.it > Subject: [LTP] [PATCH] Convert ptrace01.c to new library > > Hi, > > This is just a port of ptrace01.c. I wanted to make sure what I'm doing is right > before porting the rest. I tried to make as little changes as possible so the > test should be pretty much the same. > --- > testcases/kernel/syscalls/ptrace/ptrace01.c | 296 +++++++--------------------- > 1 file changed, 72 insertions(+), 224 deletions(-) > > diff --git a/testcases/kernel/syscalls/ptrace/ptrace01.c > b/testcases/kernel/syscalls/ptrace/ptrace01.c > index 5a7ed5ffd..ab9ef35ee 100644 > --- a/testcases/kernel/syscalls/ptrace/ptrace01.c > +++ b/testcases/kernel/syscalls/ptrace/ptrace01.c > @@ -1,36 +1,13 @@ > -/* > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > * Copyright (c) Wipro Technologies Ltd, 2002. All Rights Reserved. > * > - * 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. > + * Author: Saji Kumar.V.R <saji.kumar@wipro.com> > * > - * 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. > + * Ported to new library: > + * 10/2019 Jorik Cronenberg <jcronenberg@suse.de> > * > - * 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 IDENTIFIER : ptrace01 > - * > - * EXECUTED BY : anyone > - * > - * TEST TITLE : functionality test for ptrace(2) > - * > - * TEST CASE TOTAL : 2 > - * > - * AUTHOR : Saji Kumar.V.R <saji.kumar@wipro.com> > - * > - * SIGNALS > - * Uses SIGUSR1 to pause before test if option set. > - * (See the parse_opts(3) man page). > - * > - * DESCRIPTION > + * DESCRIPTION > * This test case tests the functionality of ptrace() for > * PTRACE_TRACEME & PTRACE_KILL requests. > * Here, we fork a child & the child does ptrace(PTRACE_TRACEME, ...). > @@ -44,243 +21,114 @@ > * In both cases, child should stop & notify parent on reception > * of SIGUSR2 > * > - * Setup: > - * Setup signal handling. > - * Pause for SIGUSR1 if option specified. > - * > - * Test: > - * Loop if the proper options are given. > - * setup signal handler for SIGUSR2 signal > - * fork a child > - * > - * CHILD: > - * setup signal handler for SIGUSR2 signal > - * call ptrace() with PTRACE_TRACEME request > - * send SIGUSR2 signal to self > - * PARENT: > - * wait() for child. > - * if parent is notified when child gets a signal through wait(), > - * then > - * do ptrace(PTRACE_KILL, ..) on child > - * wait() for child to finish, > - * if child exited abnormaly, > - * TEST passed > - * else > - * TEST failed > - * else > - * TEST failed > - * > - * Cleanup: > - * Print errno log and/or timing stats if options given > - * > - * USAGE: <for command-line> > - * ptrace01 [-c n] [-e] [-i n] [-I x] [-P x] [-t] [-h] [-f] [-p] > - * where, -c n : Run n copies concurrently. > - * -e : Turn on errno logging. > - * -h : Show help screen > - * -f : Turn off functional testing > - * -i n : Execute test n times. > - * -I x : Execute test for x seconds. > - * -p : Pause for SIGUSR1 before starting > - * -P x : Pause for x seconds between iterations. > - * -t : Turn on syscall timing. > - * > - > ********************************************************** > ******/ > + */ It looks like the old comment had some nicely structured meta-data. I'm not familiar with the process for converting to the new library. Is it intentional to eliminate this meta-data? (This is more of a question for the LTP leaders than for Jorik). I know that Cyril is working on a new meta-data system. If this meta-data is eliminated now, does it make adding it back into the new system harder? Personally, I find it useful to see the pseudo-code outline when I'm debugging a test. So I hate to see those be eliminated. But maybe those are hard to trust as they tend to not be maintained along with the code? -- Tim > > +#include <stdlib.h> > #include <errno.h> > #include <signal.h> > #include <sys/wait.h> > - > #include <config.h> > #include "ptrace.h" > +#include "tst_test.h" > > -#include "test.h" > - > -static void do_child(void); > -static void setup(void); > -static void cleanup(void); > -static void child_handler(); > -static void parent_handler(); > +static int got_signal; > > -static int got_signal = 0; > - > -char *TCID = "ptrace01"; > -static int i; /* loop test case counter, shared with > do_child */ > - > -int TST_TOTAL = 2; > - > -int main(int ac, char **av) > +void child_handler(void) > { > - > - int lc; > - pid_t child_pid; > - int status; > - struct sigaction parent_act; > - > - tst_parse_opts(ac, av, NULL, NULL); > -#ifdef UCLINUX > - maybe_run_child(&do_child, "d", &i); > -#endif > - > - setup(); > - > - for (lc = 0; TEST_LOOPING(lc); lc++) { > - > - tst_count = 0; > - > - for (i = 0; i < TST_TOTAL; ++i) { > - got_signal = 0; > - > - /* Setup signal handler for parent */ > - if (i == 1) { > - parent_act.sa_handler = parent_handler; > - parent_act.sa_flags = SA_RESTART; > - sigemptyset(&parent_act.sa_mask); > - > - if ((sigaction(SIGUSR2, &parent_act, NULL)) > - == -1) { > - tst_resm(TWARN, "sigaction() failed" > - " in parent"); > - continue; > - } > - } > - > - switch (child_pid = FORK_OR_VFORK()) { > - > - case -1: > - /* fork() failed */ > - tst_resm(TFAIL, "fork() failed"); > - continue; > - > - case 0: > - /* Child */ > -#ifdef UCLINUX > - if (self_exec(av[0], "d", i) < 0) { > - tst_resm(TFAIL, "self_exec failed"); > - continue; > - } > -#else > - do_child(); > -#endif > - > - default: > - /* Parent */ > - if ((waitpid(child_pid, &status, 0)) < 0) { > - tst_resm(TFAIL, "waitpid() failed"); > - continue; > - } > - > - /* > - * Check the exit status of child. If (it exits > - * normally with exit value 1) OR (child came > - * through signal handler), Test Failed > - */ > - > - if (((WIFEXITED(status)) && > - (WEXITSTATUS(status))) || > - (got_signal == 1)) { > - tst_resm(TFAIL, "Test Failed"); > - continue; > - } else { > - /* Kill child */ > - if ((ptrace(PTRACE_KILL, child_pid, > - 0, 0)) == -1) { > - tst_resm(TFAIL, "Test Failed:" > - " Parent was not able > to kill" > - " child"); > - continue; > - } > - } > - > - if ((waitpid(child_pid, &status, 0)) < 0) { > - tst_resm(TFAIL, "waitpid() failed"); > - continue; > - } > - > - if (WIFEXITED(status)) { > - /* Child exits normally */ > - tst_resm(TFAIL, "Test failed"); > - } else { > - tst_resm(TPASS, "Test Passed"); > - } > - > - } > - } > + if ((kill(getppid(), SIGUSR2)) == -1) { > + tst_res(TWARN, "kill() failed in child_handler()"); > + exit(1); > } > +} > > - /* cleanup and exit */ > - cleanup(); > - tst_exit(); > - > +void parent_handler(void) > +{ > + got_signal = 1; > } > > -/* do_child() */ > -void do_child(void) > +void do_child(unsigned int i) > { > struct sigaction child_act; > > - /* Setup signal handler for child */ > - if (i == 0) { > + if (i == 0) > child_act.sa_handler = SIG_IGN; > - } else { > + else > child_act.sa_handler = child_handler; > - } > + > child_act.sa_flags = SA_RESTART; > sigemptyset(&child_act.sa_mask); > > if ((sigaction(SIGUSR2, &child_act, NULL)) == -1) { > - tst_resm(TWARN, "sigaction() failed in child"); > + tst_res(TWARN, "sigaction() failed in child"); > exit(1); > } > > if ((ptrace(PTRACE_TRACEME, 0, 0, 0)) == -1) { > - tst_resm(TWARN, "ptrace() failed in child"); > + tst_res(TWARN, "ptrace() failed in child"); > exit(1); > } > - /* ensure that child bypasses signal handler */ > if ((kill(getpid(), SIGUSR2)) == -1) { > - tst_resm(TWARN, "kill() failed in child"); > + tst_res(TWARN, "kill() failed in child"); > exit(1); > } > exit(1); > } > > -/* setup() - performs all ONE TIME setup for this test */ > -void setup(void) > +static void run(unsigned int i) > { > > - tst_sig(FORK, DEF_HANDLER, cleanup); > + pid_t child_pid; > + int status; > + struct sigaction parent_act; > > - TEST_PAUSE; > +#ifdef UCLINUX > + maybe_run_child(&do_child, "d", &i); > +#endif > > -} > + got_signal = 0; > > -/* > - *cleanup() - performs all ONE TIME cleanup for this test at > - * completion or premature exit. > - */ > -void cleanup(void) > -{ > + if (i == 1) { > + parent_act.sa_handler = parent_handler; > + parent_act.sa_flags = SA_RESTART; > + sigemptyset(&parent_act.sa_mask); > > -} > + if ((sigaction(SIGUSR2, &parent_act, NULL)) > + == -1) { > + tst_res(TWARN, "sigaction() failed in parent"); > + } > + } > > -/* > - * child_handler() - Signal handler for child > - */ > -void child_handler(void) > -{ > + child_pid = SAFE_FORK(); > > - if ((kill(getppid(), SIGUSR2)) == -1) { > - tst_resm(TWARN, "kill() failed in child_handler()"); > - exit(1); > - } > -} > + if (child_pid != 0) { > > -/* > - * parent_handler() - Signal handler for parent > - */ > -void parent_handler(void) > -{ > + SAFE_WAITPID(child_pid, &status, 0); > > - got_signal = 1; > + if (((WIFEXITED(status)) && > + (WEXITSTATUS(status))) || > + (got_signal == 1)) { > + tst_res(TFAIL, "Test Failed"); > + } else { > + if ((ptrace(PTRACE_KILL, child_pid, > + 0, 0)) == -1) { > + tst_res(TFAIL, > + "Test Failed: Parent was not > able to kill child"); > + } > + } > + > + SAFE_WAITPID(child_pid, &status, 0); > + > + if (WIFEXITED(status)) > + tst_res(TFAIL, "Test failed"); > + else > + tst_res(TPASS, "Test Passed"); > + > + } else > + do_child(i); > } > + > +static struct tst_test test = { > + .test = run, > + .tcnt = 2, > + .forks_child = 1, > +}; > -- > 2.16.4 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp
Hi! > > /********************************************************** > > - * > > - * TEST IDENTIFIER : ptrace01 > > - * > > - * EXECUTED BY : anyone > > - * > > - * TEST TITLE : functionality test for ptrace(2) > > - * > > - * TEST CASE TOTAL : 2 > > - * > > - * AUTHOR : Saji Kumar.V.R <saji.kumar@wipro.com> > > - * > > - * SIGNALS > > - * Uses SIGUSR1 to pause before test if option set. > > - * (See the parse_opts(3) man page). > > - * > > - * DESCRIPTION > > + * DESCRIPTION > > * This test case tests the functionality of ptrace() for > > * PTRACE_TRACEME & PTRACE_KILL requests. > > * Here, we fork a child & the child does ptrace(PTRACE_TRACEME, ...). > > @@ -44,243 +21,114 @@ > > * In both cases, child should stop & notify parent on reception > > * of SIGUSR2 > > * > > - * Setup: > > - * Setup signal handling. > > - * Pause for SIGUSR1 if option specified. > > - * > > - * Test: > > - * Loop if the proper options are given. > > - * setup signal handler for SIGUSR2 signal > > - * fork a child > > - * > > - * CHILD: > > - * setup signal handler for SIGUSR2 signal > > - * call ptrace() with PTRACE_TRACEME request > > - * send SIGUSR2 signal to self > > - * PARENT: > > - * wait() for child. > > - * if parent is notified when child gets a signal through wait(), > > - * then > > - * do ptrace(PTRACE_KILL, ..) on child > > - * wait() for child to finish, > > - * if child exited abnormaly, > > - * TEST passed > > - * else > > - * TEST failed > > - * else > > - * TEST failed > > - * > > - * Cleanup: > > - * Print errno log and/or timing stats if options given > > - * > > - * USAGE: <for command-line> > > - * ptrace01 [-c n] [-e] [-i n] [-I x] [-P x] [-t] [-h] [-f] [-p] > > - * where, -c n : Run n copies concurrently. > > - * -e : Turn on errno logging. > > - * -h : Show help screen > > - * -f : Turn off functional testing > > - * -i n : Execute test n times. > > - * -I x : Execute test for x seconds. > > - * -p : Pause for SIGUSR1 before starting > > - * -P x : Pause for x seconds between iterations. > > - * -t : Turn on syscall timing. > > - * > > - > > ********************************************************** > > ******/ > > + */ > > It looks like the old comment had some nicely structured meta-data. I'm not > familiar with the process for converting to the new library. Is it intentional to > eliminate this meta-data? (This is more of a question for the LTP leaders > than for Jorik). Well most of the old data is unfortunatelly garbage, for instance it lists the USAGE lists flags implemented in the test library, repeating these in each test is actually doing a harm. So we tend to keep carefuly selected subset. I suspect that there was some kind of silly template that had to be filled up in order to get test accepted in the old days, which resulted in long and mostly useless comments on the top of the testcases. > I know that Cyril is working on a new meta-data system. If this meta-data is eliminated > now, does it make adding it back into the new system harder? > > Personally, I find it useful to see the pseudo-code outline when I'm debugging a > test. So I hate to see those be eliminated. But maybe those are hard to trust > as they tend to not be maintained along with the code? Well in this case the patch actually keeps the test description, which is the same as the pseudo-code but in a paragraph of a text, so as far as I can tell it's fine.
Hi! > This is just a port of ptrace01.c. I wanted to make sure what I'm doing is right before porting the rest. I tried to make as little changes as possible so the test should be pretty much the same. Generally looks good, a few comments below. Also the part before the --- should look like: Ported ptrace01 to new library. Signed-off-by: Jorik Cronenberg <jcronenberg@suse.de> > --- And the informal part about making minimal changes should be here, after the ---, which is the part that is not include in the final commit. > testcases/kernel/syscalls/ptrace/ptrace01.c | 296 +++++++--------------------- > 1 file changed, 72 insertions(+), 224 deletions(-) > > diff --git a/testcases/kernel/syscalls/ptrace/ptrace01.c b/testcases/kernel/syscalls/ptrace/ptrace01.c > index 5a7ed5ffd..ab9ef35ee 100644 > --- a/testcases/kernel/syscalls/ptrace/ptrace01.c > +++ b/testcases/kernel/syscalls/ptrace/ptrace01.c > @@ -1,36 +1,13 @@ > -/* > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > * Copyright (c) Wipro Technologies Ltd, 2002. All Rights Reserved. > * > - * 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. Being pedantic about the licences this is GPL-2.0 only and we cannot change the original licence without consent from the author, so we have to keep the code under GPL-2.0 and not the -or-later variant. > + * Author: Saji Kumar.V.R <saji.kumar@wipro.com> > * > - * 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. > + * Ported to new library: > + * 10/2019 Jorik Cronenberg <jcronenberg@suse.de> > * > - * 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 IDENTIFIER : ptrace01 > - * > - * EXECUTED BY : anyone > - * > - * TEST TITLE : functionality test for ptrace(2) > - * > - * TEST CASE TOTAL : 2 > - * > - * AUTHOR : Saji Kumar.V.R <saji.kumar@wipro.com> > - * > - * SIGNALS > - * Uses SIGUSR1 to pause before test if option set. > - * (See the parse_opts(3) man page). > - * > - * DESCRIPTION > + * DESCRIPTION > * This test case tests the functionality of ptrace() for > * PTRACE_TRACEME & PTRACE_KILL requests. > * Here, we fork a child & the child does ptrace(PTRACE_TRACEME, ...). > @@ -44,243 +21,114 @@ > * In both cases, child should stop & notify parent on reception > * of SIGUSR2 > * > - * Setup: > - * Setup signal handling. > - * Pause for SIGUSR1 if option specified. > - * > - * Test: > - * Loop if the proper options are given. > - * setup signal handler for SIGUSR2 signal > - * fork a child > - * > - * CHILD: > - * setup signal handler for SIGUSR2 signal > - * call ptrace() with PTRACE_TRACEME request > - * send SIGUSR2 signal to self > - * PARENT: > - * wait() for child. > - * if parent is notified when child gets a signal through wait(), > - * then > - * do ptrace(PTRACE_KILL, ..) on child > - * wait() for child to finish, > - * if child exited abnormaly, > - * TEST passed > - * else > - * TEST failed > - * else > - * TEST failed > - * > - * Cleanup: > - * Print errno log and/or timing stats if options given > - * > - * USAGE: <for command-line> > - * ptrace01 [-c n] [-e] [-i n] [-I x] [-P x] [-t] [-h] [-f] [-p] > - * where, -c n : Run n copies concurrently. > - * -e : Turn on errno logging. > - * -h : Show help screen > - * -f : Turn off functional testing > - * -i n : Execute test n times. > - * -I x : Execute test for x seconds. > - * -p : Pause for SIGUSR1 before starting > - * -P x : Pause for x seconds between iterations. > - * -t : Turn on syscall timing. > - * > - ****************************************************************/ > + */ > > +#include <stdlib.h> > #include <errno.h> > #include <signal.h> > #include <sys/wait.h> > - > #include <config.h> > #include "ptrace.h" > +#include "tst_test.h" > > -#include "test.h" > - > -static void do_child(void); > -static void setup(void); > -static void cleanup(void); > -static void child_handler(); > -static void parent_handler(); > +static int got_signal; Variables changed from signal handlers has to be declared volatile as well, otherwise the compiler may mis-optimize access to such variables. > -static int got_signal = 0; > - > -char *TCID = "ptrace01"; > -static int i; /* loop test case counter, shared with do_child */ > - > -int TST_TOTAL = 2; > - > -int main(int ac, char **av) > +void child_handler(void) > { > - > - int lc; > - pid_t child_pid; > - int status; > - struct sigaction parent_act; > - > - tst_parse_opts(ac, av, NULL, NULL); > -#ifdef UCLINUX > - maybe_run_child(&do_child, "d", &i); > -#endif > - > - setup(); > - > - for (lc = 0; TEST_LOOPING(lc); lc++) { > - > - tst_count = 0; > - > - for (i = 0; i < TST_TOTAL; ++i) { > - got_signal = 0; > - > - /* Setup signal handler for parent */ > - if (i == 1) { > - parent_act.sa_handler = parent_handler; > - parent_act.sa_flags = SA_RESTART; > - sigemptyset(&parent_act.sa_mask); > - > - if ((sigaction(SIGUSR2, &parent_act, NULL)) > - == -1) { > - tst_resm(TWARN, "sigaction() failed" > - " in parent"); > - continue; > - } > - } > - > - switch (child_pid = FORK_OR_VFORK()) { > - > - case -1: > - /* fork() failed */ > - tst_resm(TFAIL, "fork() failed"); > - continue; > - > - case 0: > - /* Child */ > -#ifdef UCLINUX > - if (self_exec(av[0], "d", i) < 0) { > - tst_resm(TFAIL, "self_exec failed"); > - continue; > - } > -#else > - do_child(); > -#endif > - > - default: > - /* Parent */ > - if ((waitpid(child_pid, &status, 0)) < 0) { > - tst_resm(TFAIL, "waitpid() failed"); > - continue; > - } > - > - /* > - * Check the exit status of child. If (it exits > - * normally with exit value 1) OR (child came > - * through signal handler), Test Failed > - */ > - > - if (((WIFEXITED(status)) && > - (WEXITSTATUS(status))) || > - (got_signal == 1)) { > - tst_resm(TFAIL, "Test Failed"); > - continue; > - } else { > - /* Kill child */ > - if ((ptrace(PTRACE_KILL, child_pid, > - 0, 0)) == -1) { > - tst_resm(TFAIL, "Test Failed:" > - " Parent was not able to kill" > - " child"); > - continue; > - } > - } > - > - if ((waitpid(child_pid, &status, 0)) < 0) { > - tst_resm(TFAIL, "waitpid() failed"); > - continue; > - } > - > - if (WIFEXITED(status)) { > - /* Child exits normally */ > - tst_resm(TFAIL, "Test failed"); > - } else { > - tst_resm(TPASS, "Test Passed"); > - } > - > - } > - } > + if ((kill(getppid(), SIGUSR2)) == -1) { > + tst_res(TWARN, "kill() failed in child_handler()"); > + exit(1); > } > +} Can we please use SAFE_MACROS() here? E.g. SAFE_SIGACTION(), SAFE_KILL(), ... Since with these we don't have to check the return value, which simplifies the code greatly. > - /* cleanup and exit */ > - cleanup(); > - tst_exit(); > - > +void parent_handler(void) > +{ > + got_signal = 1; > } Can we please change all the functions to static as well? I.e. the child_handler, parent_handler as well as do_child? > -/* do_child() */ > -void do_child(void) > +void do_child(unsigned int i) > { > struct sigaction child_act; > > - /* Setup signal handler for child */ > - if (i == 0) { > + if (i == 0) > child_act.sa_handler = SIG_IGN; > - } else { > + else > child_act.sa_handler = child_handler; > - } > + > child_act.sa_flags = SA_RESTART; > sigemptyset(&child_act.sa_mask); > > if ((sigaction(SIGUSR2, &child_act, NULL)) == -1) { > - tst_resm(TWARN, "sigaction() failed in child"); > + tst_res(TWARN, "sigaction() failed in child"); > exit(1); > } > > if ((ptrace(PTRACE_TRACEME, 0, 0, 0)) == -1) { > - tst_resm(TWARN, "ptrace() failed in child"); > + tst_res(TWARN, "ptrace() failed in child"); > exit(1); > } > - /* ensure that child bypasses signal handler */ > if ((kill(getpid(), SIGUSR2)) == -1) { > - tst_resm(TWARN, "kill() failed in child"); > + tst_res(TWARN, "kill() failed in child"); > exit(1); > } > exit(1); > } > > -/* setup() - performs all ONE TIME setup for this test */ > -void setup(void) > +static void run(unsigned int i) > { > > - tst_sig(FORK, DEF_HANDLER, cleanup); > + pid_t child_pid; > + int status; > + struct sigaction parent_act; > > - TEST_PAUSE; > +#ifdef UCLINUX > + maybe_run_child(&do_child, "d", &i); > +#endif We are staging out the uClinux support, feel free to delete everyting between #ifdef UCLINUX and #endif. > -} > + got_signal = 0; > > -/* > - *cleanup() - performs all ONE TIME cleanup for this test at > - * completion or premature exit. > - */ > -void cleanup(void) > -{ > + if (i == 1) { > + parent_act.sa_handler = parent_handler; > + parent_act.sa_flags = SA_RESTART; > + sigemptyset(&parent_act.sa_mask); > > -} > + if ((sigaction(SIGUSR2, &parent_act, NULL)) > + == -1) { I guess that the original code was over 80 lines which resulted into such ugly line wrapping. Can we please keep conditions on a single line unless they are over 80 lines? > + tst_res(TWARN, "sigaction() failed in parent"); > + } > + } > > -/* > - * child_handler() - Signal handler for child > - */ > -void child_handler(void) > -{ > + child_pid = SAFE_FORK(); > > - if ((kill(getppid(), SIGUSR2)) == -1) { > - tst_resm(TWARN, "kill() failed in child_handler()"); > - exit(1); > - } > -} > + if (child_pid != 0) { > > -/* > - * parent_handler() - Signal handler for parent > - */ > -void parent_handler(void) > -{ > + SAFE_WAITPID(child_pid, &status, 0); > > - got_signal = 1; > + if (((WIFEXITED(status)) && > + (WEXITSTATUS(status))) || > + (got_signal == 1)) { > + tst_res(TFAIL, "Test Failed"); > + } else { > + if ((ptrace(PTRACE_KILL, child_pid, > + 0, 0)) == -1) { > + tst_res(TFAIL, > + "Test Failed: Parent was not able to kill child"); > + } > + } Can with print errno here if the ptrace has failed? Hint: Use the TERRNO flag for tst_res(). Also the message could be shorter, we don't have to include the "Test Failed" in it since the test library already prints that because of the TFAIL flag, and we may as well be shorter and to the point with something as "ptrace(PTRACE_KILL, child_pid, ...) failed", which carries the same information. > + SAFE_WAITPID(child_pid, &status, 0); > + > + if (WIFEXITED(status)) > + tst_res(TFAIL, "Test failed"); > + else > + tst_res(TPASS, "Test Passed"); We can do better in these messages, "Test failed" is nearly meaningless since that information is carried in the TFAIL flag already. Also we should be more pedantic and pass the test only and only if it was killed. Looking at manual pages the PTRACE_KILL should send SIGKILL signal to the process being traced, so we should pass the test only and only if it was killed. Also we do have a tst_strstatus() in the test library, so that we can print what happened to the child process in case that it wasn't killed by SIGKILL. > + } else > + do_child(i); This part would be more elegant as: if (!child_pid) { do_child(); exit(1); } SAFE_WAITPID(...); ... > } > + > +static struct tst_test test = { > + .test = run, > + .tcnt = 2, > + .forks_child = 1, > +}; > -- > 2.16.4 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp
diff --git a/testcases/kernel/syscalls/ptrace/ptrace01.c b/testcases/kernel/syscalls/ptrace/ptrace01.c index 5a7ed5ffd..ab9ef35ee 100644 --- a/testcases/kernel/syscalls/ptrace/ptrace01.c +++ b/testcases/kernel/syscalls/ptrace/ptrace01.c @@ -1,36 +1,13 @@ -/* +// SPDX-License-Identifier: GPL-2.0-or-later +/* * Copyright (c) Wipro Technologies Ltd, 2002. All Rights Reserved. * - * 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. + * Author: Saji Kumar.V.R <saji.kumar@wipro.com> * - * 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. + * Ported to new library: + * 10/2019 Jorik Cronenberg <jcronenberg@suse.de> * - * 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 IDENTIFIER : ptrace01 - * - * EXECUTED BY : anyone - * - * TEST TITLE : functionality test for ptrace(2) - * - * TEST CASE TOTAL : 2 - * - * AUTHOR : Saji Kumar.V.R <saji.kumar@wipro.com> - * - * SIGNALS - * Uses SIGUSR1 to pause before test if option set. - * (See the parse_opts(3) man page). - * - * DESCRIPTION + * DESCRIPTION * This test case tests the functionality of ptrace() for * PTRACE_TRACEME & PTRACE_KILL requests. * Here, we fork a child & the child does ptrace(PTRACE_TRACEME, ...). @@ -44,243 +21,114 @@ * In both cases, child should stop & notify parent on reception * of SIGUSR2 * - * Setup: - * Setup signal handling. - * Pause for SIGUSR1 if option specified. - * - * Test: - * Loop if the proper options are given. - * setup signal handler for SIGUSR2 signal - * fork a child - * - * CHILD: - * setup signal handler for SIGUSR2 signal - * call ptrace() with PTRACE_TRACEME request - * send SIGUSR2 signal to self - * PARENT: - * wait() for child. - * if parent is notified when child gets a signal through wait(), - * then - * do ptrace(PTRACE_KILL, ..) on child - * wait() for child to finish, - * if child exited abnormaly, - * TEST passed - * else - * TEST failed - * else - * TEST failed - * - * Cleanup: - * Print errno log and/or timing stats if options given - * - * USAGE: <for command-line> - * ptrace01 [-c n] [-e] [-i n] [-I x] [-P x] [-t] [-h] [-f] [-p] - * where, -c n : Run n copies concurrently. - * -e : Turn on errno logging. - * -h : Show help screen - * -f : Turn off functional testing - * -i n : Execute test n times. - * -I x : Execute test for x seconds. - * -p : Pause for SIGUSR1 before starting - * -P x : Pause for x seconds between iterations. - * -t : Turn on syscall timing. - * - ****************************************************************/ + */ +#include <stdlib.h> #include <errno.h> #include <signal.h> #include <sys/wait.h> - #include <config.h> #include "ptrace.h" +#include "tst_test.h" -#include "test.h" - -static void do_child(void); -static void setup(void); -static void cleanup(void); -static void child_handler(); -static void parent_handler(); +static int got_signal; -static int got_signal = 0; - -char *TCID = "ptrace01"; -static int i; /* loop test case counter, shared with do_child */ - -int TST_TOTAL = 2; - -int main(int ac, char **av) +void child_handler(void) { - - int lc; - pid_t child_pid; - int status; - struct sigaction parent_act; - - tst_parse_opts(ac, av, NULL, NULL); -#ifdef UCLINUX - maybe_run_child(&do_child, "d", &i); -#endif - - setup(); - - for (lc = 0; TEST_LOOPING(lc); lc++) { - - tst_count = 0; - - for (i = 0; i < TST_TOTAL; ++i) { - got_signal = 0; - - /* Setup signal handler for parent */ - if (i == 1) { - parent_act.sa_handler = parent_handler; - parent_act.sa_flags = SA_RESTART; - sigemptyset(&parent_act.sa_mask); - - if ((sigaction(SIGUSR2, &parent_act, NULL)) - == -1) { - tst_resm(TWARN, "sigaction() failed" - " in parent"); - continue; - } - } - - switch (child_pid = FORK_OR_VFORK()) { - - case -1: - /* fork() failed */ - tst_resm(TFAIL, "fork() failed"); - continue; - - case 0: - /* Child */ -#ifdef UCLINUX - if (self_exec(av[0], "d", i) < 0) { - tst_resm(TFAIL, "self_exec failed"); - continue; - } -#else - do_child(); -#endif - - default: - /* Parent */ - if ((waitpid(child_pid, &status, 0)) < 0) { - tst_resm(TFAIL, "waitpid() failed"); - continue; - } - - /* - * Check the exit status of child. If (it exits - * normally with exit value 1) OR (child came - * through signal handler), Test Failed - */ - - if (((WIFEXITED(status)) && - (WEXITSTATUS(status))) || - (got_signal == 1)) { - tst_resm(TFAIL, "Test Failed"); - continue; - } else { - /* Kill child */ - if ((ptrace(PTRACE_KILL, child_pid, - 0, 0)) == -1) { - tst_resm(TFAIL, "Test Failed:" - " Parent was not able to kill" - " child"); - continue; - } - } - - if ((waitpid(child_pid, &status, 0)) < 0) { - tst_resm(TFAIL, "waitpid() failed"); - continue; - } - - if (WIFEXITED(status)) { - /* Child exits normally */ - tst_resm(TFAIL, "Test failed"); - } else { - tst_resm(TPASS, "Test Passed"); - } - - } - } + if ((kill(getppid(), SIGUSR2)) == -1) { + tst_res(TWARN, "kill() failed in child_handler()"); + exit(1); } +} - /* cleanup and exit */ - cleanup(); - tst_exit(); - +void parent_handler(void) +{ + got_signal = 1; } -/* do_child() */ -void do_child(void) +void do_child(unsigned int i) { struct sigaction child_act; - /* Setup signal handler for child */ - if (i == 0) { + if (i == 0) child_act.sa_handler = SIG_IGN; - } else { + else child_act.sa_handler = child_handler; - } + child_act.sa_flags = SA_RESTART; sigemptyset(&child_act.sa_mask); if ((sigaction(SIGUSR2, &child_act, NULL)) == -1) { - tst_resm(TWARN, "sigaction() failed in child"); + tst_res(TWARN, "sigaction() failed in child"); exit(1); } if ((ptrace(PTRACE_TRACEME, 0, 0, 0)) == -1) { - tst_resm(TWARN, "ptrace() failed in child"); + tst_res(TWARN, "ptrace() failed in child"); exit(1); } - /* ensure that child bypasses signal handler */ if ((kill(getpid(), SIGUSR2)) == -1) { - tst_resm(TWARN, "kill() failed in child"); + tst_res(TWARN, "kill() failed in child"); exit(1); } exit(1); } -/* setup() - performs all ONE TIME setup for this test */ -void setup(void) +static void run(unsigned int i) { - tst_sig(FORK, DEF_HANDLER, cleanup); + pid_t child_pid; + int status; + struct sigaction parent_act; - TEST_PAUSE; +#ifdef UCLINUX + maybe_run_child(&do_child, "d", &i); +#endif -} + got_signal = 0; -/* - *cleanup() - performs all ONE TIME cleanup for this test at - * completion or premature exit. - */ -void cleanup(void) -{ + if (i == 1) { + parent_act.sa_handler = parent_handler; + parent_act.sa_flags = SA_RESTART; + sigemptyset(&parent_act.sa_mask); -} + if ((sigaction(SIGUSR2, &parent_act, NULL)) + == -1) { + tst_res(TWARN, "sigaction() failed in parent"); + } + } -/* - * child_handler() - Signal handler for child - */ -void child_handler(void) -{ + child_pid = SAFE_FORK(); - if ((kill(getppid(), SIGUSR2)) == -1) { - tst_resm(TWARN, "kill() failed in child_handler()"); - exit(1); - } -} + if (child_pid != 0) { -/* - * parent_handler() - Signal handler for parent - */ -void parent_handler(void) -{ + SAFE_WAITPID(child_pid, &status, 0); - got_signal = 1; + if (((WIFEXITED(status)) && + (WEXITSTATUS(status))) || + (got_signal == 1)) { + tst_res(TFAIL, "Test Failed"); + } else { + if ((ptrace(PTRACE_KILL, child_pid, + 0, 0)) == -1) { + tst_res(TFAIL, + "Test Failed: Parent was not able to kill child"); + } + } + + SAFE_WAITPID(child_pid, &status, 0); + + if (WIFEXITED(status)) + tst_res(TFAIL, "Test failed"); + else + tst_res(TPASS, "Test Passed"); + + } else + do_child(i); } + +static struct tst_test test = { + .test = run, + .tcnt = 2, + .forks_child = 1, +};