Re: [PATCH v4] selftests/vm: fix errno handling in mrelease_test

[Suren Baghdasaryan <surenb@google.com>]

On Wed, Jul 6, 2022 at 8:29 AM David Vernet <void@manifault.com> 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@wowsignal.io>

Reviewed-by: Suren Baghdasaryan <surenb@google.com>

> > ---
> > 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@manifault.com>

Thanks for the fix!