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

[Li Wang]

Hi Petr,

On Mon, Jan 29, 2024 at 4:03 PM Petr Vorel <pvorel@suse.cz> wrote:

> > 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.
>

Good point, actually I am tired of figuring out the return 0 means 'FAIL' or
'SUCCESS' in the lib, it messes a lot in some functions. And as you suggest,
bool value should be a good way to resolve this.

But the bool type is not built into the language prior to the C99 standard.
The C99 standard introduced a _Bool type and the header <stdbool.h>,
which defines bool as a macro for _Bool.

I am not sure if LTP nowadays only supports C99 and later.
or should we make use of bool safely (or use syscalls approach
0 == success) in our patch?

@Cyril Hrubis <chrubis@suse.cz> what do you think?




> > @@ -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.
>

Agree.


-- 
Regards,
Li Wang