Re: [LTP] [PATCH v2 1/2] libswap: add two methods to create swapfile

[Petr Vorel <pvorel@suse.cz>]

Hi Li,

Generally LGTM.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

...
>  /*
> - * Make a swap file
> + * Create a swapfile of a specified size or number of blocks.
>   */
> -int make_swapfile(const char *swapfile, int blocks, int safe);
> +int make_swapfile(const char *swapfile, unsigned int num,
> +			int safe, enum swapfile_method method);
I wonder if it would help to add const char *file, const int lineno here.

> +
> +#define MAKE_SWAPFILE_SIZE(swapfile, size, safe) \
> +    make_swapfile(swapfile, size, safe, SWAPFILE_BY_SIZE)
nit: I like the name but one have to search which units (kB vs. MB vs. GB) are used.

> +
> +#define MAKE_SWAPFILE_BLKS(swapfile, blocks, safe) \
> +    make_swapfile(swapfile, blocks, safe, SWAPFILE_BY_BLKS)

And we could also have SAFE_ variants.

Therefore maybe rename make_swapfile() to make_swapfile_()
(approach in LTP for functions to be wrapped) and define macros:

int make_swapfile_(const char *file, const int lineno,
	const char *swapfile, unsigned int num,
	int safe, enum swapfile_method method);

#define MAKE_SWAPFILE_SIZE(swapfile, size, safe) \
    make_swapfile_(__FILE__, __LINE__, swapfile, size, 0, SWAPFILE_BY_SIZE)

#define MAKE_SWAPFILE_BLKS(swapfile, blocks, safe) \
    make_swapfile_(__FILE__, __LINE__, swapfile, blocks, 0, SWAPFILE_BY_BLKS)

#define SAFE_MAKE_SWAPFILE_SIZE(swapfile, size, safe) \
    make_swapfile_(__FILE__, __LINE__, swapfile, size, 1, SWAPFILE_BY_SIZE)

#define SAFE_MAKE_SWAPFILE_BLKS(swapfile, blocks, safe) \
    make_swapfile_(__FILE__, __LINE__, swapfile, blocks, 1, SWAPFILE_BY_BLKS)


>  /*
>   * Check swapon/swapoff support status of filesystems or files
> diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> index a26ea25e4..0e2476ec2 100644
> --- a/libs/libltpswap/libswap.c
> +++ b/libs/libltpswap/libswap.c
> @@ -133,18 +133,26 @@ out:
>  	return contiguous;
>  }

> -int make_swapfile(const char *swapfile, int blocks, int safe)
> +int make_swapfile(const char *swapfile, unsigned int num, int safe, enum swapfile_method method)
>  {
>  	struct statvfs fs_info;
>  	unsigned long blk_size, bs;
>  	size_t pg_size = sysconf(_SC_PAGESIZE);
>  	char mnt_path[100];
> +	unsigned int blocks = 0;

>  	if (statvfs(".", &fs_info) == -1)
>  		return -1;

>  	blk_size = fs_info.f_bsize;

> +	if (method == SWAPFILE_BY_SIZE)
> +		blocks = num * 1024 * 1024 / blk_size;
> +	else if (method == SWAPFILE_BY_BLKS)
> +		blocks = num;
> +	else
> +		tst_brk(TBROK, "Invalid method, please see include/libswap.h");

nit: I would print the method.

Using const char *file, const int lineno and tst_brk_() would help
later to point out which file actually contains wrong method.

...

Kind regards,
Petr

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