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

[Cyril Hrubis <chrubis@suse.cz>]

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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp