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

Wei Gao via ltp
2023-03-02T10:45:46Z

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);
> > }