* [LTP] [PATCH 0/2] syscalls/tkill: Convert tkill{01, 02} to the new API @ 2021-04-22 6:57 Xie Ziyao 2021-04-22 6:57 ` [LTP] [PATCH 1/2] syscalls/tkill: Convert tkill01 " Xie Ziyao 2021-04-22 6:57 ` [LTP] [PATCH 2/2] syscalls/tkill: Convert tkill02 " Xie Ziyao 0 siblings, 2 replies; 12+ messages in thread From: Xie Ziyao @ 2021-04-22 6:57 UTC (permalink / raw) To: ltp 1. Capture signals to verify success in tkill01.c, while the previous code didn't make it work; 2. Cleanup and convert tkill{01, 02} to the new API. Xie Ziyao (2): syscalls/tkill: Convert tkill01 to the new API syscalls/tkill: Convert tkill02 to the new API testcases/kernel/syscalls/tkill/tkill01.c | 122 +++++++--------------- testcases/kernel/syscalls/tkill/tkill02.c | 105 ++++++------------- 2 files changed, 70 insertions(+), 157 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [PATCH 1/2] syscalls/tkill: Convert tkill01 to the new API 2021-04-22 6:57 [LTP] [PATCH 0/2] syscalls/tkill: Convert tkill{01, 02} to the new API Xie Ziyao @ 2021-04-22 6:57 ` Xie Ziyao 2021-04-26 10:31 ` Petr Vorel 2021-04-22 6:57 ` [LTP] [PATCH 2/2] syscalls/tkill: Convert tkill02 " Xie Ziyao 1 sibling, 1 reply; 12+ messages in thread From: Xie Ziyao @ 2021-04-22 6:57 UTC (permalink / raw) To: ltp 1. Convert tkill01 to the new API; 2. Capture signals to verify success, while the previous code didn't make it work. Signed-off-by: Xie Ziyao <xieziyao@huawei.com> --- testcases/kernel/syscalls/tkill/tkill01.c | 122 +++++++--------------- 1 file changed, 38 insertions(+), 84 deletions(-) diff --git a/testcases/kernel/syscalls/tkill/tkill01.c b/testcases/kernel/syscalls/tkill/tkill01.c index 20c28f1bc..094b0d942 100644 --- a/testcases/kernel/syscalls/tkill/tkill01.c +++ b/testcases/kernel/syscalls/tkill/tkill01.c @@ -1,42 +1,18 @@ -/******************************************************************************/ -/* Copyright (c) Crackerjack Project., 2007 */ -/* */ -/* This program is free software; you can redistribute it and/or modify */ -/* it under the terms of the GNU General Public License as published by */ -/* the Free Software Foundation; either version 2 of the License, or */ -/* (at your option) any later version. */ -/* */ -/* This program is distributed in the hope that it will be useful, */ -/* but WITHOUT ANY WARRANTY; without even the implied warranty of */ -/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See */ -/* the GNU General Public License for more details. */ -/* */ -/* You should have received a copy of the GNU General Public License */ -/* along with this program; if not, write to the Free Software */ -/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ -/* */ -/******************************************************************************/ -/******************************************************************************/ -/* */ -/* File: tkill01.c */ -/* */ -/* Description: This tests the tkill() syscall */ -/* */ -/* Usage: <for command-line> */ -/* tkill01 [-c n] [-e][-i n] [-I x] [-p x] [-t] */ -/* where, -c n : Run n copies concurrently. */ -/* -e : Turn on errno logging. */ -/* -i n : Execute test n times. */ -/* -I x : Execute test for x seconds. */ -/* -P x : Pause for x seconds between iterations. */ -/* -t : Turn on syscall timing. */ -/* */ -/* Total Tests: 1 */ -/* */ -/* Test Name: tkill01 */ -/* History: Porting from Crackerjack to LTP is done by */ -/* Manas Kumar Nayak maknayak@in.ibm.com> */ -/******************************************************************************/ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) Crackerjack Project., 2007 + * Ported from Crackerjack to LTP by Manas Kumar Nayak maknayak@in.ibm.com> + */ + +/*\ + * [Description] + * + * Basic tests for the tkill syscall. + * + * [Algorithm] + * + * Calls tkill and capture signals to verify success. + */ #include <stdio.h> #include <stdlib.h> @@ -48,59 +24,37 @@ #include <linux/unistd.h> #include <sys/types.h> -#include "test.h" #include "lapi/syscalls.h" +#include "tst_test.h" -char *TCID = "tkill01"; -int testno; -int TST_TOTAL = 2; +int sig_flag = 0; -void cleanup(void) +static void sighandler(int sig) { - - tst_rmdir(); -} - -void setup(void) -{ - TEST_PAUSE; - tst_tmpdir(); + if (sig == SIGUSR1) + sig_flag = 1; } -int sig_count = 0; - -void sig_action(int sig) -{ - sig_count = 1; -} - -int main(int ac, char **av) +static void run(void) { int tid; - int lc; - - tst_parse_opts(ac, av, NULL, NULL); - setup(); - - for (lc = 0; TEST_LOOPING(lc); ++lc) { - tst_count = 0; - for (testno = 0; testno < TST_TOTAL; ++testno) { - if (signal(SIGUSR1, &sig_action) == SIG_ERR) - tst_brkm(TBROK | TERRNO, cleanup, - "signal(SIGUSR1, ..) failed"); - TEST(tid = ltp_syscall(__NR_gettid)); - if (TEST_RETURN == -1) { - tst_resm(TFAIL | TTERRNO, "tkill failed"); - } - TEST(ltp_syscall(__NR_tkill, tid, SIGUSR1)); - if (TEST_RETURN == 0) { - tst_resm(TPASS, "tkill call succeeded"); - } else { - tst_resm(TFAIL | TTERRNO, "tkill failed"); - } - } + SAFE_SIGNAL(SIGUSR1, sighandler); + TEST(tid = tst_syscall(__NR_gettid)); + if (TST_RET == -1) + tst_res(TFAIL | TTERRNO, "tst_syscall(__NR_gettid) failed"); + + TEST(tst_syscall(__NR_tkill, tid, SIGUSR1)); + if (TST_RET == 0) { + while (!sig_flag); + tst_res(TPASS, "tst_syscall(__NR_tkill, %d, SIGUSR1)", tid); + } else { + tst_res(TFAIL | TTERRNO, + "tst_syscall(__NR_tkill, %d, SIGUSR1)", tid); } - cleanup(); - tst_exit(); } + +static struct tst_test test = { + .needs_tmpdir = 1, + .test_all = run, +}; -- 2.17.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [LTP] [PATCH 1/2] syscalls/tkill: Convert tkill01 to the new API 2021-04-22 6:57 ` [LTP] [PATCH 1/2] syscalls/tkill: Convert tkill01 " Xie Ziyao @ 2021-04-26 10:31 ` Petr Vorel 2021-04-26 11:24 ` xieziyao 2021-04-28 12:11 ` Cyril Hrubis 0 siblings, 2 replies; 12+ messages in thread From: Petr Vorel @ 2021-04-26 10:31 UTC (permalink / raw) To: ltp Hi, > 1. Convert tkill01 to the new API; > 2. Capture signals to verify success, while the previous code > didn't make it work. Generally LGTM, with comments below. Reviewed-by: Petr Vorel <pvorel@suse.cz> > #include <stdio.h> > #include <stdlib.h> > @@ -48,59 +24,37 @@ > #include <linux/unistd.h> > #include <sys/types.h> I removed these as not needed. The only one which is still relevant is <signal.h> (I kept it although it's not needed to be included, as it's included in tst_safe_macros.h which are included in tst_test.h). > -#include "test.h" > #include "lapi/syscalls.h" > +#include "tst_test.h" > +int sig_flag = 0; It should be static int sig_flag; ... > +static void run(void) ... > + SAFE_SIGNAL(SIGUSR1, sighandler); > + TEST(tid = tst_syscall(__NR_gettid)); > + if (TST_RET == -1) > + tst_res(TFAIL | TTERRNO, "tst_syscall(__NR_gettid) failed"); gettid() manpage says "ERRORS: This call is alway successful". I suppose this is true also for raw syscall. And it's certainly true for tst_syscall(__NR_gettid). BTW if it really needed to be checked, tst_brk() or tst_res() with return would need to be used. > + > + TEST(tst_syscall(__NR_tkill, tid, SIGUSR1)); > + if (TST_RET == 0) { > + while (!sig_flag); Not sure why you required this. > + tst_res(TPASS, "tst_syscall(__NR_tkill, %d, SIGUSR1)", tid); > + } else { > + tst_res(TFAIL | TTERRNO, > + "tst_syscall(__NR_tkill, %d, SIGUSR1)", tid); > } > - cleanup(); > - tst_exit(); > } > + > +static struct tst_test test = { > + .needs_tmpdir = 1, > + .test_all = run, > +}; In the end going to merge code below. Kind regards, Petr // SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (c) Linux Test Project, 2009-2021 * Copyright (c) Crackerjack Project., 2007 * Ported from Crackerjack to LTP by Manas Kumar Nayak maknayak@in.ibm.com> */ /*\ * [Description] * * Basic tests for the tkill syscall. * * [Algorithm] * * Calls tkill and capture signals to verify success. */ #include <signal.h> #include "lapi/syscalls.h" #include "tst_test.h" static int sig_flag; static void sighandler(int sig) { if (sig == SIGUSR1) sig_flag = 1; } static void run(void) { int tid; SAFE_SIGNAL(SIGUSR1, sighandler); tid = tst_syscall(__NR_gettid); TST_EXP_PASS(tst_syscall(__NR_tkill, tid, SIGUSR1)); } static struct tst_test test = { .needs_tmpdir = 1, .test_all = run, }; ^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [PATCH 1/2] syscalls/tkill: Convert tkill01 to the new API 2021-04-26 10:31 ` Petr Vorel @ 2021-04-26 11:24 ` xieziyao 2021-04-26 12:55 ` Petr Vorel 2021-04-28 12:11 ` Cyril Hrubis 1 sibling, 1 reply; 12+ messages in thread From: xieziyao @ 2021-04-26 11:24 UTC (permalink / raw) To: ltp Hi, Thanks for your review, Petr. > + TEST(tst_syscall(__NR_tkill, tid, SIGUSR1)); > + if (TST_RET == 0) { > + while (!sig_flag); This while loop is written to check whether the sighandler function captures the SIGUSR1 signal and set sig_flag to 1. > + tst_res(TPASS, "tst_syscall(__NR_tkill, %d, SIGUSR1)", tid); > + } else { > + tst_res(TFAIL | TTERRNO, > + "tst_syscall(__NR_tkill, %d, SIGUSR1)", tid); > } > - cleanup(); > - tst_exit(); > } Other comments are fine to me. Best Regards, Ziyao -----Original Message----- From: Petr Vorel [mailto:pvorel@suse.cz] Sent: Monday, April 26, 2021 6:32 PM To: xieziyao <xieziyao@huawei.com> Cc: ltp@lists.linux.it Subject: Re: [LTP] [PATCH 1/2] syscalls/tkill: Convert tkill01 to the new API Hi, > 1. Convert tkill01 to the new API; > 2. Capture signals to verify success, while the previous code didn't > make it work. Generally LGTM, with comments below. Reviewed-by: Petr Vorel <pvorel@suse.cz> > #include <stdio.h> > #include <stdlib.h> > @@ -48,59 +24,37 @@ > #include <linux/unistd.h> > #include <sys/types.h> I removed these as not needed. The only one which is still relevant is <signal.h> (I kept it although it's not needed to be included, as it's included in tst_safe_macros.h which are included in tst_test.h). > -#include "test.h" > #include "lapi/syscalls.h" > +#include "tst_test.h" > +int sig_flag = 0; It should be static int sig_flag; ... > +static void run(void) ... > + SAFE_SIGNAL(SIGUSR1, sighandler); > + TEST(tid = tst_syscall(__NR_gettid)); > + if (TST_RET == -1) > + tst_res(TFAIL | TTERRNO, "tst_syscall(__NR_gettid) failed"); gettid() manpage says "ERRORS: This call is alway successful". I suppose this is true also for raw syscall. And it's certainly true for tst_syscall(__NR_gettid). BTW if it really needed to be checked, tst_brk() or tst_res() with return would need to be used. > + > + TEST(tst_syscall(__NR_tkill, tid, SIGUSR1)); > + if (TST_RET == 0) { > + while (!sig_flag); Not sure why you required this. > + tst_res(TPASS, "tst_syscall(__NR_tkill, %d, SIGUSR1)", tid); > + } else { > + tst_res(TFAIL | TTERRNO, > + "tst_syscall(__NR_tkill, %d, SIGUSR1)", tid); > } > - cleanup(); > - tst_exit(); > } > + > +static struct tst_test test = { > + .needs_tmpdir = 1, > + .test_all = run, > +}; In the end going to merge code below. Kind regards, Petr // SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (c) Linux Test Project, 2009-2021 * Copyright (c) Crackerjack Project., 2007 * Ported from Crackerjack to LTP by Manas Kumar Nayak maknayak@in.ibm.com> */ /*\ * [Description] * * Basic tests for the tkill syscall. * * [Algorithm] * * Calls tkill and capture signals to verify success. */ #include <signal.h> #include "lapi/syscalls.h" #include "tst_test.h" static int sig_flag; static void sighandler(int sig) { if (sig == SIGUSR1) sig_flag = 1; } static void run(void) { int tid; SAFE_SIGNAL(SIGUSR1, sighandler); tid = tst_syscall(__NR_gettid); TST_EXP_PASS(tst_syscall(__NR_tkill, tid, SIGUSR1)); } static struct tst_test test = { .needs_tmpdir = 1, .test_all = run, }; ^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [PATCH 1/2] syscalls/tkill: Convert tkill01 to the new API 2021-04-26 11:24 ` xieziyao @ 2021-04-26 12:55 ` Petr Vorel 2021-04-27 1:49 ` xieziyao 0 siblings, 1 reply; 12+ messages in thread From: Petr Vorel @ 2021-04-26 12:55 UTC (permalink / raw) To: ltp > Hi, > Thanks for your review, Petr. > > + TEST(tst_syscall(__NR_tkill, tid, SIGUSR1)); > > + if (TST_RET == 0) { > > + while (!sig_flag); > This while loop is written to check whether the sighandler function captures the SIGUSR1 signal and set sig_flag to 1. Oh, correct. But not sure if it's good to use plain while. Maybe using usleep(1000) in while loop? TST_EXP_PASS(tst_syscall(__NR_tkill, tid, SIGUSR1)); while (!sig_flag) usleep(1000); Kind regards, Petr > > + tst_res(TPASS, "tst_syscall(__NR_tkill, %d, SIGUSR1)", tid); > > + } else { > > + tst_res(TFAIL | TTERRNO, > > + "tst_syscall(__NR_tkill, %d, SIGUSR1)", tid); > > } > > - cleanup(); > > - tst_exit(); > > } ^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [PATCH 1/2] syscalls/tkill: Convert tkill01 to the new API 2021-04-26 12:55 ` Petr Vorel @ 2021-04-27 1:49 ` xieziyao 0 siblings, 0 replies; 12+ messages in thread From: xieziyao @ 2021-04-27 1:49 UTC (permalink / raw) To: ltp Hi, Petr, Cyril, I think adding usleep(1000) in while loop is a good idea, thanks. Best Regards, Ziyao -----Original Message----- From: Petr Vorel [mailto:pvorel@suse.cz] Sent: Monday, April 26, 2021 8:55 PM To: xieziyao <xieziyao@huawei.com> Cc: ltp@lists.linux.it; Cyril Hrubis <chrubis@suse.cz> Subject: Re: [LTP] [PATCH 1/2] syscalls/tkill: Convert tkill01 to the new API > Hi, > Thanks for your review, Petr. > > + TEST(tst_syscall(__NR_tkill, tid, SIGUSR1)); > > + if (TST_RET == 0) { > > + while (!sig_flag); > This while loop is written to check whether the sighandler function captures the SIGUSR1 signal and set sig_flag to 1. Oh, correct. But not sure if it's good to use plain while. Maybe using usleep(1000) in while loop? TST_EXP_PASS(tst_syscall(__NR_tkill, tid, SIGUSR1)); while (!sig_flag) usleep(1000); Kind regards, Petr > > + tst_res(TPASS, "tst_syscall(__NR_tkill, %d, SIGUSR1)", tid); > > + } else { > > + tst_res(TFAIL | TTERRNO, > > + "tst_syscall(__NR_tkill, %d, SIGUSR1)", tid); > > } > > - cleanup(); > > - tst_exit(); > > } ^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [PATCH 1/2] syscalls/tkill: Convert tkill01 to the new API 2021-04-26 10:31 ` Petr Vorel 2021-04-26 11:24 ` xieziyao @ 2021-04-28 12:11 ` Cyril Hrubis 2021-04-28 18:04 ` Petr Vorel 1 sibling, 1 reply; 12+ messages in thread From: Cyril Hrubis @ 2021-04-28 12:11 UTC (permalink / raw) To: ltp Hi! > > -#include "test.h" > > #include "lapi/syscalls.h" > > +#include "tst_test.h" > > > +int sig_flag = 0; > > It should be > static int sig_flag; It has to be volatile as well, if we are waiting in a bussy loop on it and it's changed ansynchronously from a signal handler, otherwise compiler may misoptimize the code. Generally the code that waits for a signal should look like: static volatile sig_atomic_t sig_flag; static void setup(void) { SAFE_SIGNAL(SIGUSR1, sighandler); } static void run(void) { int timeout_ms = 1000; sig_flag = 0; TST_EXP_PASS(tst_syscall(__NR_tkill, tid, SIGUSR1)); while (timeout_ms--) { if (sig_flag) break; usleep(1000); } if (sig_flag) tst_res(TPASS, ...); else tst_res(TFAIL, ...); } -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [PATCH 1/2] syscalls/tkill: Convert tkill01 to the new API 2021-04-28 12:11 ` Cyril Hrubis @ 2021-04-28 18:04 ` Petr Vorel 2021-04-29 2:02 ` xieziyao 0 siblings, 1 reply; 12+ messages in thread From: Petr Vorel @ 2021-04-28 18:04 UTC (permalink / raw) To: ltp Hi Cyril, Xie, > > > +int sig_flag = 0; > > It should be > > static int sig_flag; > It has to be volatile as well, if we are waiting in a bussy loop on it > and it's changed ansynchronously from a signal handler, otherwise > compiler may misoptimize the code. > Generally the code that waits for a signal should look like: > static volatile sig_atomic_t sig_flag; Oh, yes, volatile. Thanks for other hints, code adjusted and whole patchset merged. Kind regards, Petr ^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [PATCH 1/2] syscalls/tkill: Convert tkill01 to the new API 2021-04-28 18:04 ` Petr Vorel @ 2021-04-29 2:02 ` xieziyao 0 siblings, 0 replies; 12+ messages in thread From: xieziyao @ 2021-04-29 2:02 UTC (permalink / raw) To: ltp Hi, Petr, Cyril, Learned a lot. Thanks for your review and modifications to the code. Kind regards, Ziyao -----Original Message----- From: Petr Vorel [mailto:pvorel@suse.cz] Sent: Thursday, April 29, 2021 2:05 AM To: Cyril Hrubis <chrubis@suse.cz> Cc: xieziyao <xieziyao@huawei.com>; ltp@lists.linux.it Subject: Re: [LTP] [PATCH 1/2] syscalls/tkill: Convert tkill01 to the new API Hi Cyril, Xie, > > > +int sig_flag = 0; > > It should be > > static int sig_flag; > It has to be volatile as well, if we are waiting in a bussy loop on it > and it's changed ansynchronously from a signal handler, otherwise > compiler may misoptimize the code. > Generally the code that waits for a signal should look like: > static volatile sig_atomic_t sig_flag; Oh, yes, volatile. Thanks for other hints, code adjusted and whole patchset merged. Kind regards, Petr ^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [PATCH 2/2] syscalls/tkill: Convert tkill02 to the new API 2021-04-22 6:57 [LTP] [PATCH 0/2] syscalls/tkill: Convert tkill{01, 02} to the new API Xie Ziyao 2021-04-22 6:57 ` [LTP] [PATCH 1/2] syscalls/tkill: Convert tkill01 " Xie Ziyao @ 2021-04-22 6:57 ` Xie Ziyao 2021-04-26 11:12 ` Petr Vorel 1 sibling, 1 reply; 12+ messages in thread From: Xie Ziyao @ 2021-04-22 6:57 UTC (permalink / raw) To: ltp Cleanup and convert tkill02 to the new API. Signed-off-by: Xie Ziyao <xieziyao@huawei.com> --- testcases/kernel/syscalls/tkill/tkill02.c | 105 +++++++--------------- 1 file changed, 32 insertions(+), 73 deletions(-) diff --git a/testcases/kernel/syscalls/tkill/tkill02.c b/testcases/kernel/syscalls/tkill/tkill02.c index 48431755b..20b461705 100644 --- a/testcases/kernel/syscalls/tkill/tkill02.c +++ b/testcases/kernel/syscalls/tkill/tkill02.c @@ -1,28 +1,18 @@ -/****************************************************************************** - * Copyright (c) Crackerjack Project., 2007 * - * * - * This program is free software; you can redistribute it and/or modify * - * it under the terms of the GNU General Public License as published by * - * the Free Software Foundation; either version 2 of the License, or * - * (at your option) any later version. * - * * - * This program is distributed in the hope that it will be useful, * - * but WITHOUT ANY WARRANTY; without even the implied warranty of * - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See * - * the GNU General Public License for more details. * - * * - * You should have received a copy of the GNU General Public License * - * along with this program; if not, write to the Free Software Foundation, * - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA * - * * - ******************************************************************************/ +// SPDX-License-Identifier: GPL-2.0-or-later /* - * File: tkill02.c + * Copyright (c) Crackerjack Project., 2007 + * Ported from Crackerjack to LTP by Manas Kumar Nayak maknayak@in.ibm.com> + */ + +/*\ + * [Description] + * + * Basic tests for the tkill errors. * - * Description: This tests the tkill() syscall + * [Algorithm] * - * History: Porting from Crackerjack to LTP is done by - * Manas Kumar Nayak maknayak@in.ibm.com> + * - EINVAL on an invalid thread ID + * - ESRCH when no process with the specified thread ID exists */ #include <stdio.h> @@ -32,66 +22,35 @@ #include <signal.h> #include <sys/syscall.h> -#include "test.h" #include "lapi/syscalls.h" +#include "tst_test.h" -char *TCID = "tkill02"; -int testno; - +static pid_t expired_pid; static pid_t inval_tid = -1; -static pid_t unused_tid; - -void cleanup(void) -{ - tst_rmdir(); -} - -void setup(void) -{ - TEST_PAUSE; - tst_tmpdir(); - - unused_tid = tst_get_unused_pid(cleanup); -} struct test_case_t { int *tid; int exp_errno; -} test_cases[] = { - {&inval_tid, EINVAL}, - {&unused_tid, ESRCH} + const char *desc; +} tc[] = { + {&inval_tid, EINVAL, "inval_tid"}, + {&expired_pid, ESRCH, "expired_pid"} }; -int TST_TOTAL = sizeof(test_cases) / sizeof(test_cases[0]); - -int main(int ac, char **av) +static void setup(void) { - int i; - - setup(); - - tst_parse_opts(ac, av, NULL, NULL); - - for (i = 0; i < TST_TOTAL; i++) { - - TEST(ltp_syscall(__NR_tkill, *(test_cases[i].tid), SIGUSR1)); + expired_pid = tst_get_unused_pid(); +} - if (TEST_RETURN == -1) { - if (TEST_ERRNO == test_cases[i].exp_errno) { - tst_resm(TPASS | TTERRNO, - "tkill(%d, SIGUSR1) failed as expected", - *(test_cases[i].tid)); - } else { - tst_brkm(TFAIL | TTERRNO, cleanup, - "tkill(%d, SIGUSR1) failed unexpectedly", - *(test_cases[i].tid)); - } - } else { - tst_brkm(TFAIL, cleanup, - "tkill(%d) succeeded unexpectedly", - *(test_cases[i].tid)); - } - } - cleanup(); - tst_exit(); +static void run(unsigned int i) +{ + TST_EXP_FAIL(tst_syscall(__NR_tkill, *(tc[i].tid), SIGUSR1), + tc[i].exp_errno, "tst_syscall(__NR_tkill, %s)", tc[i].desc); } + +static struct tst_test test = { + .tcnt = ARRAY_SIZE(tc), + .needs_tmpdir = 1, + .setup = setup, + .test = run, +}; -- 2.17.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [LTP] [PATCH 2/2] syscalls/tkill: Convert tkill02 to the new API 2021-04-22 6:57 ` [LTP] [PATCH 2/2] syscalls/tkill: Convert tkill02 " Xie Ziyao @ 2021-04-26 11:12 ` Petr Vorel 2021-04-26 11:35 ` xieziyao 0 siblings, 1 reply; 12+ messages in thread From: Petr Vorel @ 2021-04-26 11:12 UTC (permalink / raw) To: ltp Hi, LGTM with very minor changes. Reviewed-by: Petr Vorel <pvorel@suse.cz> > +static pid_t expired_pid; > static pid_t inval_tid = -1; > -static pid_t unused_tid; IMHO unused_tid is better describe what the variable holds. > - > -void cleanup(void) > -{ > - tst_rmdir(); > -} > - > -void setup(void) > -{ > - TEST_PAUSE; > - tst_tmpdir(); > - > - unused_tid = tst_get_unused_pid(cleanup); > -} > struct test_case_t { > int *tid; > int exp_errno; > -} test_cases[] = { > - {&inval_tid, EINVAL}, > - {&unused_tid, ESRCH} > + const char *desc; > +} tc[] = { > + {&inval_tid, EINVAL, "inval_tid"}, > + {&expired_pid, ESRCH, "expired_pid"} Well, there is no point to print variable name. Better would be "invalid TID" and "unused TID". But IMHO just writing what we expect is enough. It could be: #define ERRNO_DESC(x) .exp_errno = x, .desc = "exp" #x ... {&inval_tid, ERRNO_DESC(EINVAL}, {&expired_pid, ERRNO_DESC(ESRCH} But we have tst_strerrno(), thus just: struct test_case_t { int *tid; int exp_errno; } tc[] = { {&inval_tid, EINVAL}, {&unused_tid, ESRCH} }; ... TST_EXP_FAIL(tst_syscall(__NR_tkill, *(tc[i].tid), SIGUSR1), tc[i].exp_errno, "tst_syscall(__NR_tkill) expecting %s", tst_strerrno(tc[i].exp_errno)); I'll merge code below. Kind regards, Petr // SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (c) Crackerjack Project., 2007 * Ported from Crackerjack to LTP by Manas Kumar Nayak maknayak@in.ibm.com> */ /*\ * [Description] * * Basic tests for the tkill() errors. * * [Algorithm] * * - EINVAL on an invalid thread ID * - ESRCH when no process with the specified thread ID exists */ #include <stdio.h> #include <stdlib.h> #include <errno.h> #include <unistd.h> #include <signal.h> #include <sys/syscall.h> #include "lapi/syscalls.h" #include "tst_test.h" static pid_t unused_tid; static pid_t inval_tid = -1; struct test_case_t { int *tid; int exp_errno; } tc[] = { {&inval_tid, EINVAL}, {&unused_tid, ESRCH} }; static void setup(void) { unused_tid = tst_get_unused_pid(); } static void run(unsigned int i) { TST_EXP_FAIL(tst_syscall(__NR_tkill, *(tc[i].tid), SIGUSR1), tc[i].exp_errno, "tst_syscall(__NR_tkill) expecting %s", tst_strerrno(tc[i].exp_errno)); } static struct tst_test test = { .tcnt = ARRAY_SIZE(tc), .needs_tmpdir = 1, .setup = setup, .test = run, }; ^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [PATCH 2/2] syscalls/tkill: Convert tkill02 to the new API 2021-04-26 11:12 ` Petr Vorel @ 2021-04-26 11:35 ` xieziyao 0 siblings, 0 replies; 12+ messages in thread From: xieziyao @ 2021-04-26 11:35 UTC (permalink / raw) To: ltp Hi, Petr, LGTM, thanks for your review. Best Regards, Ziyao -----Original Message----- From: Petr Vorel [mailto:pvorel@suse.cz] Sent: Monday, April 26, 2021 7:12 PM To: xieziyao <xieziyao@huawei.com> Cc: ltp@lists.linux.it Subject: Re: [LTP] [PATCH 2/2] syscalls/tkill: Convert tkill02 to the new API Hi, LGTM with very minor changes. Reviewed-by: Petr Vorel <pvorel@suse.cz> > +static pid_t expired_pid; > static pid_t inval_tid = -1; > -static pid_t unused_tid; IMHO unused_tid is better describe what the variable holds. > - > -void cleanup(void) > -{ > - tst_rmdir(); > -} > - > -void setup(void) > -{ > - TEST_PAUSE; > - tst_tmpdir(); > - > - unused_tid = tst_get_unused_pid(cleanup); > -} > struct test_case_t { > int *tid; > int exp_errno; > -} test_cases[] = { > - {&inval_tid, EINVAL}, > - {&unused_tid, ESRCH} > + const char *desc; > +} tc[] = { > + {&inval_tid, EINVAL, "inval_tid"}, > + {&expired_pid, ESRCH, "expired_pid"} Well, there is no point to print variable name. Better would be "invalid TID" and "unused TID". But IMHO just writing what we expect is enough. It could be: #define ERRNO_DESC(x) .exp_errno = x, .desc = "exp" #x ... {&inval_tid, ERRNO_DESC(EINVAL}, {&expired_pid, ERRNO_DESC(ESRCH} But we have tst_strerrno(), thus just: struct test_case_t { int *tid; int exp_errno; } tc[] = { {&inval_tid, EINVAL}, {&unused_tid, ESRCH} }; ... TST_EXP_FAIL(tst_syscall(__NR_tkill, *(tc[i].tid), SIGUSR1), tc[i].exp_errno, "tst_syscall(__NR_tkill) expecting %s", tst_strerrno(tc[i].exp_errno)); I'll merge code below. Kind regards, Petr // SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (c) Crackerjack Project., 2007 * Ported from Crackerjack to LTP by Manas Kumar Nayak maknayak@in.ibm.com> */ /*\ * [Description] * * Basic tests for the tkill() errors. * * [Algorithm] * * - EINVAL on an invalid thread ID * - ESRCH when no process with the specified thread ID exists */ #include <stdio.h> #include <stdlib.h> #include <errno.h> #include <unistd.h> #include <signal.h> #include <sys/syscall.h> #include "lapi/syscalls.h" #include "tst_test.h" static pid_t unused_tid; static pid_t inval_tid = -1; struct test_case_t { int *tid; int exp_errno; } tc[] = { {&inval_tid, EINVAL}, {&unused_tid, ESRCH} }; static void setup(void) { unused_tid = tst_get_unused_pid(); } static void run(unsigned int i) { TST_EXP_FAIL(tst_syscall(__NR_tkill, *(tc[i].tid), SIGUSR1), tc[i].exp_errno, "tst_syscall(__NR_tkill) expecting %s", tst_strerrno(tc[i].exp_errno)); } static struct tst_test test = { .tcnt = ARRAY_SIZE(tc), .needs_tmpdir = 1, .setup = setup, .test = run, }; ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-04-29 2:02 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-22 6:57 [LTP] [PATCH 0/2] syscalls/tkill: Convert tkill{01, 02} to the new API Xie Ziyao 2021-04-22 6:57 ` [LTP] [PATCH 1/2] syscalls/tkill: Convert tkill01 " Xie Ziyao 2021-04-26 10:31 ` Petr Vorel 2021-04-26 11:24 ` xieziyao 2021-04-26 12:55 ` Petr Vorel 2021-04-27 1:49 ` xieziyao 2021-04-28 12:11 ` Cyril Hrubis 2021-04-28 18:04 ` Petr Vorel 2021-04-29 2:02 ` xieziyao 2021-04-22 6:57 ` [LTP] [PATCH 2/2] syscalls/tkill: Convert tkill02 " Xie Ziyao 2021-04-26 11:12 ` Petr Vorel 2021-04-26 11:35 ` xieziyao
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.