* [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 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 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 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 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 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
* [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
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.