diff mbox series

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

Message ID 20230109032700.7959-1-wegao@suse.com
State Superseded
Headers show
Series [v1] readahead02.c fixes: use tst_parse_filesize() so that we can pass sizes with units e.g. -s 128M | expand

Commit Message

coolgw Jan. 9, 2023, 3:27 a.m. UTC
Signed-off-by: WEI GAO <wegao@suse.com>
---
 testcases/kernel/syscalls/readahead/readahead02.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Richard Palethorpe Jan. 10, 2023, 10:01 a.m. UTC | #1
Hello,

coolgw <coolgw1126@gmail.com> writes:

> 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)) {
> +		tst_brk(TBROK, "invalid initial filesize '%s'", opt_fsizestr);

So far correct; if parsing fails we call tst_brk which redirects to cleanup.

>  		tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));

However this will never be called because it is on the error path.

The time which we give the test to execute is proportional to the file
size (if the size is non-default!). If it were not then large file sizes
would cause timeouts (possibly at random) which would result in
regressions.

Also note that in the existing test we only use tst_set_max_runtime when
the value is non default. This is a mistake, but luckily the
test.max_runtime = 30 and the calculated runtime for the default size
would be 33. So you are not likely to cause a regression by changing
this.

Setting to changes requested in patchwork.

>  	}
>  
> -- 
> 2.35.3
diff mbox series

Patch

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)) {
+		tst_brk(TBROK, "invalid initial filesize '%s'", opt_fsizestr);
 		tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
 	}