Re: [LTP] [PATCH v2 2/5] swapon03: Cleanup

Cyril Hrubis
2025-11-11T13:04:10Z

Hi!
> - Remove unused return code in check_and_swapoff() and setup_swap(). The
> purpose was to run cleanup() in the end of the setup() if creating
> swap fails, but return code is always 0. Also cleanup() should be run
> when test exits with tst_brk() anyway.
> - Change return code from TFAIL to TBROK.
> - Add missing brackets.
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> The same as in v1.
> I can split this if you like.
>
> testcases/kernel/syscalls/swapon/swapon03.c | 23 +++++++--------------
> 1 file changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/swapon/swapon03.c b/testcases/kernel/syscalls/swapon/swapon03.c
> index 01a3b6d8ac..d9822c01ef 100644
> --- a/testcases/kernel/syscalls/swapon/swapon03.c
> +++ b/testcases/kernel/syscalls/swapon/swapon03.c
> @@ -25,7 +25,7 @@
>
> static int swapfiles;
>
> -static int setup_swap(void)
> +static void setup_swap(void)
> {
> pid_t pid;
> int status;
> @@ -54,33 +54,27 @@ static int setup_swap(void)
> TST_EXP_PASS_SILENT(swapon(filename, 0));
> }
> exit(0);
> - } else
> + } else {
> waitpid(pid, &status, 0);
> + }
>
> if (WEXITSTATUS(status))
> - tst_brk(TFAIL, "Failed to setup swap files");
> + tst_brk(TBROK, "Failed to setup swap files");

I wonder why do we run this code in a child process to begin with? This
is probably some leftover from the conversion to the new library. Among
other things this masks proper results propagation from the
MAKE_SMALL_SWAPFILE() because there is at least one tst_brk(TCONF, "")
in there that will be converted to TBROK here.

I guess that we want to remove the fork() here and we want SAFE_SWAPON()
instead of the TST_EXP_PASS_SILENT() so that we do tst_brk() if we fail
to setup the swapfile too. The TST_EXP_PASS_SILENT() does not end the
test on a failure, which is what we want here instead.

--
Cyril Hrubis
chrubis@suse.cz