Re: [LTP] [PATCH v1] readahead02.c fixes: use tst_parse_filesize() so that we can pass sizes with units e.g. -s 128M

[Petr Vorel <pvorel@suse.cz>]

Hi Wei,

I set the other 3 mesages as supperseded in patchwor.
Please before sending email, check whether it has been received by ML.
You should get it to your mail you use as Signed-off-by:,
also it's after ~1 min in https://lists.linux.it/pipermail/ltp/
and in https://patchwork.ozlabs.org/project/ltp/list/.
After some time (few minutes) it's also in
https://lore.kernel.org/ltp/

Patch subject is just too long and yet not describing things correctly.
https://chris.beams.io/posts/git-commit/

It could be something like this (patch continues on 3rd line):

readahead02.c: allow to pass human readable sizes

e.g. ./readahead02 -s 128M

---
NOTE: it's not a fix, but enhancement.
The fix would be to fix a real problem, e.g. test allows to define an invalid
size:

$ sudo ./readahead02 -s 100
tst_device.c:93: TINFO: Found free device 4 '/dev/loop4'
tst_test.c:1093: TINFO: Formatting /dev/loop4 with ext2 opts='' extra opts=''
mke2fs 1.46.5 (30-Dec-2021)
tst_test.c:1558: TINFO: Timeout per run is 0h 01m 00s
tst_test.c:1566: TINFO: Updating max runtime to 0h 00m 01s
tst_test.c:1558: TINFO: Timeout per run is 0h 00m 31s
readahead02.c:387: TINFO: readahead length: 4096
readahead02.c:223: TINFO: Test #0: readahead on file
readahead02.c:128: TINFO: creating test file of size: 100
readahead02.c:191: TBROK: mmap((nil),0,1,32769,3,0) failed: EINVAL (22)

=> although I haven't looked if this is a bug elsewhere in a test which should
be fixed, or test just needs more size (likely) and thus size should be required
higher.

> Signed-off-by: WEI GAO <wegao@suse.com>
> ---
>  testcases/kernel/syscalls/readahead/readahead02.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

> diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
> index 3ed88c005..c282b4d68 100644
> --- a/testcases/kernel/syscalls/readahead/readahead02.c
> +++ b/testcases/kernel/syscalls/readahead/readahead02.c
> @@ -367,8 +367,8 @@ static void setup_readahead_length(void)

>  static void setup(void)
>  {
> -	if (opt_fsizestr) {
> -		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
> +        if (tst_parse_filesize(opt_fsizestr, &testfile_size, 1, INT_MAX)) {

Running make introduces warnings (we don't want to introduce new warnings):

readahead02.c: In function ‘setup’:
readahead02.c:370:46: warning: passing argument 2 of ‘tst_parse_filesize’ from incompatible pointer type [-Wincompatible-pointer-types]
  370 |         if (tst_parse_filesize(opt_fsizestr, &testfile_size, 1, INT_MAX)) {
      |                                              ^~~~~~~~~~~~~~
      |                                              |
      |                                              size_t * {aka long unsigned int *}
In file included from readahead02.c:33:
../../../../include/tst_test.h:138:52: note: expected ‘long long int *’ but argument is of type ‘size_t *’ {aka ‘long unsigned int *’}
  138 | int tst_parse_filesize(const char *str, long long *val, long long min, long long max);
      |                                         ~~~~~~~~~~~^~~

 tst_parse_filesize would have to be long long.
Therefore it would have to be casted (size_t) on other places
which expected the original size_t type.

Kind regards,
Petr

> +		tst_brk(TBROK, "invalid initial filesize '%s'", opt_fsizestr);
>  		tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
>  	}

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