On Wed, Jul 6, 2022 at 8:29 AM David Vernet <void@xxxxxxxxxxxxx> wrote:
>
> On Wed, Jul 06, 2022 at 04:16:02PM +0200, Adam Sindelar wrote:
> > mrelease_test should return KSFT_SKIP when process_mrelease is not
> > defined, but due to a perror call consuming the errno, it returns
> > KSFT_FAIL.
> >
> > This patch decides the exit codes before calling perror.
> >
> > Fixes: 33776141b812 ("selftests: vm: add process_mrelease tests")
> >
> > Signed-off-by: Adam Sindelar <adam@xxxxxxxxxxxx>
Reviewed-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
> > ---
> > v1->v2: Fixed second instance in the same file
> > v2->v3: Fixed remaining instances of errno mishandling
> > v3->v4: Added a "Fixes:" line for the log
> >
> > tools/testing/selftests/vm/mrelease_test.c | 16 +++++++++++-----
> > 1 file changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/testing/selftests/vm/mrelease_test.c b/tools/testing/selftests/vm/mrelease_test.c
> > index 96671c2f7d48..6c62966ab5db 100644
> > --- a/tools/testing/selftests/vm/mrelease_test.c
> > +++ b/tools/testing/selftests/vm/mrelease_test.c
> > @@ -62,19 +62,22 @@ static int alloc_noexit(unsigned long nr_pages, int pipefd)
> > /* The process_mrelease calls in this test are expected to fail */
> > static void run_negative_tests(int pidfd)
> > {
> > + int res;
> > /* Test invalid flags. Expect to fail with EINVAL error code. */
> > if (!syscall(__NR_process_mrelease, pidfd, (unsigned int)-1) ||
> > errno != EINVAL) {
> > + res = (errno == ENOSYS ? KSFT_SKIP : KSFT_FAIL);
> > perror("process_mrelease with wrong flags");
> > - exit(errno == ENOSYS ? KSFT_SKIP : KSFT_FAIL);
> > + exit(res);
> > }
> > /*
> > * Test reaping while process is alive with no pending SIGKILL.
> > * Expect to fail with EINVAL error code.
> > */
> > if (!syscall(__NR_process_mrelease, pidfd, 0) || errno != EINVAL) {
> > + res = (errno == ENOSYS ? KSFT_SKIP : KSFT_FAIL);
> > perror("process_mrelease on a live process");
> > - exit(errno == ENOSYS ? KSFT_SKIP : KSFT_FAIL);
> > + exit(res);
> > }
> > }
> >
> > @@ -100,8 +103,9 @@ int main(void)
> >
> > /* Test a wrong pidfd */
> > if (!syscall(__NR_process_mrelease, -1, 0) || errno != EBADF) {
> > + res = (errno == ENOSYS ? KSFT_SKIP : KSFT_FAIL);
> > perror("process_mrelease with wrong pidfd");
> > - exit(errno == ENOSYS ? KSFT_SKIP : KSFT_FAIL);
> > + exit(res);
> > }
> >
> > /* Start the test with 1MB child memory allocation */
> > @@ -156,8 +160,9 @@ int main(void)
> > run_negative_tests(pidfd);
> >
> > if (kill(pid, SIGKILL)) {
> > + res = (errno == ENOSYS ? KSFT_SKIP : KSFT_FAIL);
> > perror("kill");
> > - exit(errno == ENOSYS ? KSFT_SKIP : KSFT_FAIL);
> > + exit(res);
> > }
> >
> > success = (syscall(__NR_process_mrelease, pidfd, 0) == 0);
> > @@ -172,9 +177,10 @@ int main(void)
> > if (errno == ESRCH) {
> > retry = (size <= MAX_SIZE_MB);
> > } else {
> > + res = (errno == ENOSYS ? KSFT_SKIP : KSFT_FAIL);
> > perror("process_mrelease");
> > waitpid(pid, NULL, 0);
> > - exit(errno == ENOSYS ? KSFT_SKIP : KSFT_FAIL);
> > + exit(res);
> > }
> > }
> >
> > --
> > 2.35.1
> >
>
> Thanks, Adam. Looks great.
>
> Reviewed-by: David Vernet <void@xxxxxxxxxxxxx>
Thanks for the fix!
