Re: [LTP] [PATCH v5 8/8] libswap: Refactor is_swap_supported function to return status

[Petr Vorel <pvorel@suse.cz>]

> This updates the is_swap_supported function in the libltpswap
> to return an integer status instead of void, allowing the function
> to communicate success or failure to the caller. It introduces
> checks and returns 0 on various failure conditions while logging
> the failure without aborting the test case.

> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
>  include/libswap.h         |  2 +-
>  libs/libltpswap/libswap.c | 28 ++++++++++++++++++----------
>  2 files changed, 19 insertions(+), 11 deletions(-)

> diff --git a/include/libswap.h b/include/libswap.h
> index e67d65756..1e09db031 100644
> --- a/include/libswap.h
> +++ b/include/libswap.h
> @@ -20,5 +20,5 @@ int make_swapfile(const char *swapfile, int blocks, int safe);
>   * Check swapon/swapoff support status of filesystems or files
>   * we are testing on.
>   */
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Thanks for further improving it. Few comments to fix old issues, when we are at
it (and plan to merge after the release). But feel free to ignore.

BTW do you plan to use it somewhere?

testcases/kernel/syscalls/swapoff/swapoff01.c

nit: I would comment here the return code.
Also maybe use bool (from stdbool.h)? The advantage is that we don't think what
the return code is (sometimes we use syscalls approach when 0 is success,
otherwise - like here - 0 is failure). Slowly converting to bool would be the
best.

> -void is_swap_supported(const char *filename);
> +int is_swap_supported(const char *filename);
>  #endif /* __LIBSWAP_H__ */
> diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> index 8aecad48d..14b7d9e3a 100644
> --- a/libs/libltpswap/libswap.c
> +++ b/libs/libltpswap/libswap.c
> @@ -168,7 +168,7 @@ int make_swapfile(const char *swapfile, int blocks, int safe)
>   * Check swapon/swapoff support status of filesystems or files
>   * we are testing on.
>   */
nit: Although I did not like doc being just at the header, I now prefer it now
to have it just at the header because we don't have to sync the same text on two
places.

Kind regards,
Petr

> -void is_swap_supported(const char *filename)
> +int is_swap_supported(const char *filename)
>  {
>  	int i, sw_support = 0;
>  	int ret = make_swapfile(filename, 10, 1);
...

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