| Message ID | 20220706095707.7539-1-adam@wowsignal.io (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] selftests/vm: fix errno handling in mrelease_test | expand |
On Wed, Jul 06, 2022 at 11:57:07AM +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. As requested on the v2 version of the patch, please also include a "Fixes" line here (see [0]): Fixes: 33776141b812 ("selftests: vm: add process_mrelease tests") [0]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes > > Signed-off-by: Adam Sindelar <adam@wowsignal.io> > --- > v1->v2: Fixed second instance in the same file > v2->v3: Fixed remaining instances of errno mishandling > > 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 [...] Looks good otherwise, thanks. Reviewed-by: David Vernet <void@manifault.com>
On Wed, Jul 06, 2022 at 06:45:08AM -0700, David Vernet wrote: > On Wed, Jul 06, 2022 at 11:57:07AM +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. > > As requested on the v2 version of the patch, please also include a "Fixes" line > here (see [0]): > > Fixes: 33776141b812 ("selftests: vm: add process_mrelease tests") > > [0]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes > Done, sending shortly. Sorry, getting used to review over email. > > > > Signed-off-by: Adam Sindelar <adam@wowsignal.io> > > --- > > v1->v2: Fixed second instance in the same file > > v2->v3: Fixed remaining instances of errno mishandling > > > > 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 > > [...] > > Looks good otherwise, thanks. > > Reviewed-by: David Vernet <void@manifault.com>
On Wed, Jul 06, 2022 at 04:15:07PM +0200, Adam Sindelar wrote:
> Done, sending shortly. Sorry, getting used to review over email.
Thank you! And no worries at all, it can be a bit of an arcane process, and
takes some getting used to.
- David
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); } }
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. Signed-off-by: Adam Sindelar <adam@wowsignal.io> --- v1->v2: Fixed second instance in the same file v2->v3: Fixed remaining instances of errno mishandling tools/testing/selftests/vm/mrelease_test.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)