Re: [LTP] [PATCH v7] fsconfig03: SKIP check return value for old kernel

[Wei Gao via ltp <ltp@lists.linux.it>]

On Thu, Mar 02, 2023 at 11:00:08AM +0100, Petr Vorel wrote:
> Hi Wei,
> 
> > Signed-off-by: Wei Gao <wegao@suse.com>
> > ---
> >  .../kernel/syscalls/fsconfig/fsconfig03.c     | 21 ++++++++++++-------
> >  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> > diff --git a/testcases/kernel/syscalls/fsconfig/fsconfig03.c b/testcases/kernel/syscalls/fsconfig/fsconfig03.c
> > index 7ee37f4ae..9adf06207 100644
> > --- a/testcases/kernel/syscalls/fsconfig/fsconfig03.c
> > +++ b/testcases/kernel/syscalls/fsconfig/fsconfig03.c
> > @@ -41,15 +41,20 @@ static void run(void)
> >  	if (pagesize == -1)
> >  		tst_brk(TBROK, "sysconf(_SC_PAGESIZE) failed");
> 
> > -	for (size_t i = 0; i < 5000; i++) {
> > -		/* use same logic in kernel legacy_parse_param function */
> > -		const size_t len = i * (strlen(val) + 2) + (strlen(val) + 1) + 2;
> > +	if ((tst_kvercmp(5, 17, 1)) >= 0) {
> I suppose 722d94847de29 (in .tags) change the old behavior (from v5.17-rc1).
> Shouldn't be the check against 5.17.0?
> if ((tst_kvercmp(5, 17, 0)) >= 0) {
yes, 5.17.0 is better one!
> 
> 
> > +		for (size_t i = 0; i < 5000; i++) {
> > +			/* use same logic in kernel legacy_parse_param function */
> > +			const size_t len = i * (strlen(val) + 2) + (strlen(val) + 1) + 2;
> 
> > -		if (!strcmp(tst_device->fs_type, "btrfs") && len <= (size_t)pagesize)
> > -			TST_EXP_PASS_SILENT(fsconfig(fd, FSCONFIG_SET_STRING, "\x00", val, 0));
> > -		else
> > -			TST_EXP_FAIL_SILENT(fsconfig(fd, FSCONFIG_SET_STRING, "\x00", val, 0),
> > -					    EINVAL);
> > +			if (!strcmp(tst_device->fs_type, "btrfs") && len <= (size_t)pagesize)
> > +				TST_EXP_PASS_SILENT(fsconfig(fd, FSCONFIG_SET_STRING, "\x00", val, 0));
> > +			else
> > +				TST_EXP_FAIL_SILENT(fsconfig(fd, FSCONFIG_SET_STRING, "\x00", val, 0),
> > +						EINVAL);
> > +		}
> > +	} else {
> > +		for (size_t i = 0; i < 5000; i++)
> Repeating the loop again. Wouldn't be more readable moving the if clause to
> separate function and doing if/else inside of for loop?
> 
> Also "\x00" might be in #define (used 3 times).
> 
> Kind regards,
> Petr
> 
> > +			fsconfig(fd, FSCONFIG_SET_STRING, "\x00", val, 0);
> >  	}

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