[LTP] [PATCH v2 1/2] ptrace05: Refactor the test using new LTP API
Andrea Cervesato
andrea.cervesato@suse.com
Thu Feb 8 17:15:49 CET 2024
Hi!
On 12/1/23 01:59, Wei Gao via ltp wrote:
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
> testcases/kernel/syscalls/ptrace/ptrace05.c | 179 ++++++--------------
> 1 file changed, 50 insertions(+), 129 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/ptrace/ptrace05.c
> b/testcases/kernel/syscalls/ptrace/ptrace05.c
> index 541018393..267d5f9d2 100644
> --- a/testcases/kernel/syscalls/ptrace/ptrace05.c
> +++ b/testcases/kernel/syscalls/ptrace/ptrace05.c
> @@ -1,178 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> /*
> -
> ******************************************************************************
> - *
> - * ptrace05 - an app which ptraces itself as per arbitrarily
> specified signals,
> - * over a user specified range.
> - *
> - * Copyright (C) 2009, Ngie Cooper
> - *
> - * 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.
> + * Copyright (c) Linux Test Project, 2009-2019
> + * Copyright (C) 2009, Ngie Cooper
> + * Copyright (c) 2023 Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
> *
> - * 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.
> + * This test ptraces itself as per arbitrarily specified signals,
> + * over 0 to SIGRTMAX range.
> *
> -
> ******************************************************************************
> */
>
> -#include <sys/types.h>
> -#include <sys/wait.h>
> -#include <signal.h>
> -#include <errno.h>
> -#include <libgen.h>
> -#include <math.h>
> #include <stdlib.h>
> -#include <stdio.h>
> -#include <string.h>
> -#include <unistd.h>
> #include <sys/ptrace.h>
> -
> -#include "test.h"
> #include "lapi/signal.h"
> +#include "tst_test.h"
>
> -char *TCID = "ptrace05";
> -int TST_TOTAL = 0;
> -
> -int usage(const char *);
> -
> -int usage(const char *argv0)
> -{
> - fprintf(stderr, "usage: %s [start-signum] [end-signum]\n", argv0);
> - return 1;
> -}
> -
> -int main(int argc, char **argv)
> +static void run(void)
> {
>
new line to remove
> - int end_signum = -1;
> - int signum;
> - int start_signum = -1;
> + int signum = 0;
> int status;
>
new line to remove
> pid_t child;
>
> - tst_parse_opts(argc, argv, NULL, NULL);
> -
> - if (start_signum == -1) {
> - start_signum = 0;
> - }
> - if (end_signum == -1) {
> - end_signum = SIGRTMAX;
> - }
> -
> - for (signum = start_signum; signum <= end_signum; signum++) {
> -
> + for (signum = 0; signum <= SIGRTMAX; signum++) {
> if (signum >= __SIGRTMIN && signum < SIGRTMIN)
> continue;
>
> - switch (child = fork()) {
> - case -1:
> - tst_brkm(TBROK | TERRNO, NULL, "fork() failed");
> - case 0:
> + child = SAFE_FORK();
>
> - if (ptrace(PTRACE_TRACEME, 0, NULL, NULL) != -1) {
> - tst_resm(TINFO, "[child] Sending kill(.., %d)",
> - signum);
> - if (kill(getpid(), signum) < 0) {
> - tst_resm(TINFO | TERRNO,
> - "[child] kill(.., %d) failed.",
> - signum);
> - }
> + if (child == 0) {
> + TEST(ptrace(PTRACE_TRACEME, 0, NULL, NULL));
> + if (TST_RET != -1) {
> + tst_res(TINFO, "[child] Sending kill(.., %d)",
> + signum);
> + SAFE_KILL(getpid(), signum);
> } else {
> -
> - /*
> - * This won't increment the TST_COUNT var.
> - * properly, but it'll show up as a failure
> - * nonetheless.
> - */
> - tst_resm(TFAIL | TERRNO,
> - "Failed to ptrace(PTRACE_TRACEME, ...) "
> - "properly");
> -
> + tst_brk(TFAIL | TERRNO,
> + "Failed to ptrace(PTRACE_TRACEME, ...) "
> + "properly");
> }
> +
> /* Shouldn't get here if signum == 0. */
> exit((signum == 0 ? 0 : 2));
> - break;
> -
> - default:
> -
> - waitpid(child, &status, 0);
> + } else {
I would remove the "else" statement, because we don't really need it
once we handle the child and it creates more indentation.
> + SAFE_WAITPID(child, &status, 0);
>
> switch (signum) {
> case 0:
> if (WIFEXITED(status)
> && WEXITSTATUS(status) == 0) {
> - tst_resm(TPASS,
> - "kill(.., 0) exited "
> - "with 0, as expected.");
> + tst_res(TPASS,
> + "kill(.., 0) exited with 0, as expected.");
> } else {
> - tst_resm(TFAIL,
> - "kill(.., 0) didn't exit "
> - "with 0.");
> + tst_brk(TFAIL | TERRNO,
> + "kill(.., 0) didn't exit with 0.");
> }
> break;
> case SIGKILL:
> if (WIFSIGNALED(status)) {
> /* SIGKILL must be uncatchable. */
> if (WTERMSIG(status) == SIGKILL) {
> - tst_resm(TPASS,
> - "Killed with SIGKILL, "
> - "as expected.");
> + tst_res(TPASS,
> + "Killed with SIGKILL, as expected.");
> } else {
> - tst_resm(TPASS,
> - "Didn't die with "
> - "SIGKILL (?!) ");
> + tst_brk(TFAIL | TERRNO,
> + "Didn't die with SIGKILL (?!) ");
> }
> } else if (WIFEXITED(status)) {
> - tst_resm(TFAIL,
> - "Exited unexpectedly instead "
> - "of dying with SIGKILL.");
> + tst_res(TFAIL | TERRNO,
> + "Exited unexpectedly instead of dying with
> SIGKILL.");
> } else if (WIFSTOPPED(status)) {
> - tst_resm(TFAIL,
> - "Stopped instead of dying "
> - "with SIGKILL.");
> + tst_res(TFAIL | TERRNO,
> + "Stopped instead of dying with SIGKILL.");
> }
> break;
> /* All other processes should be stopped. */
> default:
> - if (WIFSTOPPED(status)) {
> - tst_resm(TPASS, "Stopped as expected");
> - } else {
> - tst_resm(TFAIL, "Didn't stop as "
> - "expected.");
> - if (kill(child, 0)) {
> - tst_resm(TINFO,
> - "Is still alive!?");
> - } else if (WIFEXITED(status)) {
> - tst_resm(TINFO,
> - "Exited normally");
> - } else if (WIFSIGNALED(status)) {
> - tst_resm(TINFO,
> - "Was signaled with "
> - "signum=%d",
> - WTERMSIG(status));
> - }
> -
> - }
> -
> + if (WIFSTOPPED(status))
> + tst_res(TPASS, "Stopped as expected");
> + else
> + tst_res(TFAIL | TERRNO, "Didn't stop as expected.");
> break;
> -
> }
>
new line to remove :-) I know it can be boring, but please keep new line
where it's needed.
> }
> - /* Make sure the child dies a quick and painless death ... */
> - kill(child, 9);
> -
new line to add
> + if (signum != 0 && signum != SIGKILL)
> + SAFE_PTRACE(PTRACE_CONT, child, NULL, NULL);
> }
> -
> - tst_exit();
> -
> }
> +
> +static struct tst_test test = {
> + .test_all = run,
> + .forks_child = 1,
> +};
Besides a few stylish things, it's fine.
Regards,
Andrea Cervesato
More information about the ltp
mailing list