[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <85ce18a6-9783-427b-bfb4-64b82c0081cb@linuxfoundation.org>
Date: Fri, 5 Jul 2024 11:39:53 -0600
From: Shuah Khan <skhan@...uxfoundation.org>
To: Abdulrasaq Lawani <abdulrasaqolawani@...il.com>,
Shuah Khan <shuah@...nel.org>
Cc: javiercarrascocruz@...il.com, linux-kernel@...r.kernel.org,
linux-kselftest@...r.kernel.org, Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH v2] selftest: acct: Add selftest for the acct() syscall
On 6/30/24 13:22, Abdulrasaq Lawani wrote:
> Noticed that there was no selftest for the acct() syscall
> which enables the kernel to record terminated processes
> into a specified file.
>
> The acct() system call enables or disables process accounting.
> If accounting is turned on, records for each terminating process
> are appended to a specified filename as it terminates. An argument of NULL
> causes accounting to be turned off.
>
> This patch provides a test for the acct() syscall.
>
> References:
> https://man7.org/linux/man-pages/man2/acct.2.html
>
> Signed-off-by: Abdulrasaq Lawani <abdulrasaqolawani@...il.com>
What does the test output look like?
> ---
> Changes in v2:
> Add testcases to test error conditions.
> Add kselftest function for reporting results.
>
> - Link to v1: https://lore.kernel.org/r/20240622-kselftest-acct-syscall-v1-1-d270b5be8d37@gmail.com
> ---
> tools/testing/selftests/Makefile | 1 +
> tools/testing/selftests/acct/.gitignore | 2 +
> tools/testing/selftests/acct/Makefile | 4 ++
> tools/testing/selftests/acct/acct_syscall.c | 89 +++++++++++++++++++++++++++++
> 4 files changed, 96 insertions(+)
>
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 9039f3709aff..45a58ef5ad92 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -1,4 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0
> +TARGETS += acct
> TARGETS += alsa
> TARGETS += amd-pstate
> TARGETS += arm64
> diff --git a/tools/testing/selftests/acct/.gitignore b/tools/testing/selftests/acct/.gitignore
> new file mode 100644
> index 000000000000..8ab358d81bd2
> --- /dev/null
> +++ b/tools/testing/selftests/acct/.gitignore
> @@ -0,0 +1,2 @@
> +acct_syscall
> +config
> \ No newline at end of file
What is this?
> diff --git a/tools/testing/selftests/acct/Makefile b/tools/testing/selftests/acct/Makefile
> new file mode 100644
> index 000000000000..ff3e238c5634
> --- /dev/null
> +++ b/tools/testing/selftests/acct/Makefile
> @@ -0,0 +1,4 @@
> +TEST_GEN_PROGS := acct_syscall
> +CFLAGS += -Wall
> +
> +include ../lib.mk
> \ No newline at end of file
> diff --git a/tools/testing/selftests/acct/acct_syscall.c b/tools/testing/selftests/acct/acct_syscall.c
> new file mode 100644
> index 000000000000..4fa00a88a1bd
> --- /dev/null
> +++ b/tools/testing/selftests/acct/acct_syscall.c
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/* kselftest for acct() system call
> + * The acct() system call enables or disables process accounting.
> + */
> +
> +#include <stdio.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <sys/wait.h>
> +
> +#include "../kselftest.h"
> +
> +int main(void)
> +{
> + // Setting up kselftest framework
> + ksft_print_header();
> + ksft_set_plan(1);
> +
> + // Create file to log closed processes
> + char filename[] = "process_log";
Where does this file created?
> + FILE *fp;
> +
> + fp = fopen(filename, "w");
What happens if file creation fails?
> +
> + int i = acct(filename);
Chose a descriptive name for the variable e.g: ret
> +
> + // Handle error conditions
> + if (i) {
Is this the failure case - ret < 0 case - complete conditional.
> + switch (errno) {
> + case EPERM:
> + ksft_test_result_error("%s. Please run the test as root.\n",
> + strerror(errno));
> + break;
> +
> + case EACCES:
> + ksft_test_result_error("Insufficient privilege.\n");
> + break;
> +
> + case EIO:
> + ksft_test_result_error("Error writing to the file: %s.\n", filename);
> + break;
> +
> + default:
> + ksft_test_result_error("%s.\n", strerror(errno));
> + break;
> + }
> +
> + remove(filename);
> + fclose(fp);
> + ksft_finished();
> + return 1;
> + }
> +
> + // Create child process and wait for it to terminate.
> + pid_t child_pid;
> +
> + child_pid = fork();
> +
> + if (child_pid < 0) {
> + ksft_test_result_error("Process failed\n");
> + ksft_finished();
> + return 1;
> + } else if (child_pid == 0) {
> + ksft_print_msg("Child process successfully created!\n");
You don't need braces here since it it a single statement after the
conditional.
> + } else {
> + wait(NULL);
> + fseek(fp, 0L, SEEK_END);
> + int sz = ftell(fp);
> +
> + ksft_print_msg("Parent process successfully created!\n");
> +
> + i = acct(NULL);
> +
> + if (sz <= 0) {
> + ksft_test_result_fail("Terminated child process not logged");
> + ksft_exit_fail();
> + return 1;
> + }
> +
> + ksft_test_result_pass("Successfully logged terminated process.\n");
> + remove(filename);
> + fclose(fp);
> + ksft_exit_pass();
> + return 0;
> + }
> +
> + return 1;
> +}
>
> ---
> base-commit: 50736169ecc8387247fe6a00932852ce7b057083
> change-id: 20240622-kselftest-acct-syscall-2d90f7666b1e
>
> Best regards,
Please check coding guidelines and ruun checkpatch on this.
Powered by blists - more mailing lists