Re: [LTP] [PATCH v3 3/4] tree: Use TST_EXP_PASS_OR_FAIL() and TST_EXP_FD_OR_FAIL()

Hi Andrea, all,

> > > Hi Petr,

> > > > +	TST_EXP_FD_OR_FAIL(fd = open(FILE_TRUNCATE, O_WRONLY | O_TRUNC, PERM_MODE), exp_err);

> > > the correct usage should be:

> > > fd = TST_EXP_FD_OR_FAIL(open(FILE_TRUNCATE, O_WRONLY | O_TRUNC, PERM_MODE), exp_err);

@Andrea I suppose the reason why you or somebody else set whole patch as changes
requested was the line above. Once you have time, would you please have look at
the changes I proposed in my previous mail?

Kind regards,
Petr

> > Just a note: both compile and works the same (i.e. both are correct), but of
> > course the variant you suggested (fd = TST_EXP_FD_OR_FAIL...) is clearer.

> > This is because newer macros use ({  ...  TST_RET; }). But the original macro
> > TEST() use do { ... } while (0), that's why we slightly ugly code:

> > $ git grep 'TEST.*(fd ='
> > testcases/kernel/syscalls/fsconfig/fsconfig01.c:        TEST(fd = fsopen(tst_device->fs_type, 0));
> > testcases/kernel/syscalls/fsconfig/fsconfig02.c:        TEST(fd = fsopen(tst_device->fs_type, 0));
> > testcases/kernel/syscalls/fsconfig/fsconfig03.c:        TEST(fd = fsopen(tst_device->fs_type, 0));
> > testcases/kernel/syscalls/fsmount/fsmount02.c:  TEST(fd = fsopen(tst_device->fs_type, 0));
> > testcases/kernel/syscalls/fsopen/fsopen01.c:    TEST(fd = fsopen(tst_device->fs_type, tc->flags));
> > testcases/kernel/syscalls/move_mount/move_mount01.c:    TEST(fd = fsopen(tst_device->fs_type, 0));
> > testcases/kernel/syscalls/move_mount/move_mount02.c:    TEST(fd = fsopen(tst_device->fs_type, 0));
> > testcases/kernel/syscalls/mq_open/mq_open01.c:  TEST(fd = mq_open(qname, tc->oflag, S_IRWXU, tc->rq));
> > testcases/kernel/syscalls/name_to_handle_at/name_to_handle_at01.c:      TEST(fd = open_by_handle_at(*tc->dfd, fhp, tc->flags));
> > testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at01.c:      TEST(fd = open_by_handle_at(*tc->dfd, *tc->fhp, tc->flags));
> > testcases/kernel/syscalls/open_by_handle_at/open_by_handle_at02.c:      TEST(fd = open_by_handle_at(tc->dfd, *tc->fhp, tc->flags));
> > testcases/kernel/syscalls/openat2/openat201.c:  TEST(fd = openat2(*tc->dfd, tc->pathname, myhow, tc->size));
> > testcases/kernel/syscalls/socket/socket01.c:    TEST(fd = socket(tc->domain, tc->type, tc->proto));

> > Anyway, this could be fixed before merge if this is a single thing to fix, but I
> > worry about agents findings:
> > https://lore.kernel.org/ltp/[email protected]/

> Therefore I suggest to merge with changes below (removed O_TRUNC and move fd =
> to the beginning).

> > Kind regards,
> > Petr

> +++ testcases/kernel/syscalls/landlock/landlock_tester.h
> @@ -254,13 +254,13 @@ static void _test_truncate(const int exp_err)
>  	tst_res(TINFO, "Test truncating file");

>  	TST_EXP_PASS_OR_FAIL(truncate(FILE_TRUNCATE, 10), exp_err);
> -	TST_EXP_FD_OR_FAIL(fd = open(FILE_TRUNCATE, O_WRONLY | O_TRUNC, PERM_MODE), exp_err);
> +	fd = TST_EXP_FD_OR_FAIL(open(FILE_TRUNCATE, O_WRONLY, PERM_MODE), exp_err);
>  	if (fd != -1) {
>  		TST_EXP_PASS_OR_FAIL(ftruncate(fd, 10), exp_err);
>  		SAFE_CLOSE(fd);
>  	}

> -	TST_EXP_FD_OR_FAIL(fd = open(FILE_TRUNCATE, O_WRONLY | O_TRUNC, PERM_MODE), exp_err);
> +	fd = TST_EXP_FD_OR_FAIL(open(FILE_TRUNCATE, O_WRONLY | O_TRUNC, PERM_MODE), exp_err);
>  	if (fd != -1)
>  		SAFE_CLOSE(TST_RET);
>  }

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