[PATCH v2 00/10] selftests: Make ksft_exit functions return void instead of int

[PATCH v2 00/10] selftests: Make ksft_exit functions return void instead of int

[Nathan Chancellor]

Hi all,

Commit f7d5bcd35d42 ("selftests: kselftest: Mark functions that
unconditionally call exit() as __noreturn") marked functions that call
exit() as __noreturn but it did not change the return type of these
functions from 'void' to 'int' like it should have (since a noreturn
function by definition cannot return an integer because it does not
return...) because there are many tests that return the result of the
ksft_exit function, even though it has never been used due to calling
exit().

Prior to adding __noreturn, the compiler would not know that the functions
that call exit() will not return, so code like

  void ksft_exit_fail(void)
  {
    exit(1);
  }

  void ksft_exit_pass(void)
  {
    exit(0);
  }

  int main(void)
  {
    int ret;

    ret = foo();
    if (ret)
      ksft_exit_fail();
    ksft_exit_pass();
  }

would cause the compiler to complain that main() does not return an
integer, even though when ksft_exit_pass() is called, exit() will cause
the program to terminate. So ksft_exit_...() returns int to make the
compiler happy.

  int ksft_exit_fail(void)
  {
    exit(1);
  }

  int ksft_exit_pass(void)
  {
    exit(0);
  }

  int main(void)
  {
    int ret;

    ret = foo();
    if (ret)
      return ksft_exit_fail();
    return ksft_exit_pass();
  }

While this results in no warnings, it is weird semantically and it has
issues as noted in the aforementioned __noreturn change. Now that
__noreturn has been added to these functions, it is much cleaner to
change the functions to 'void' and eliminate the return statements, as
it has been made clear to the compiler that these functions terminate
the program. Drop the return before all instances of ksft_exit_...() in
a mostly mechanical way.

---
Changes in v2:
- Split series into individual patches per subsystem at Shuah's
  request and CC maintainers for subsystems that have one.
- Rewrite commit messages for new patches and move previous commit
  message into cover letter to high level explain all changes.
- Carry forward Thomas and Muhammad's review on patch split, as there
  were no functional changes, please holler if this was inappropriate.
- Link to v1: https://lore.kernel.org/r/20240417-ksft-exit-int-to-void-v1-1-eff48fdbab39@kernel.org

---
Nathan Chancellor (10):
      selftests/clone3: ksft_exit functions do not return
      selftests/ipc: ksft_exit functions do not return
      selftests: membarrier: ksft_exit_pass() does not return
      selftests/mm: ksft_exit functions do not return
      selftests: pidfd: ksft_exit functions do not return
      selftests/resctrl: ksft_exit_skip() does not return
      selftests: sync: ksft_exit_pass() does not return
      selftests: timers: ksft_exit functions do not return
      selftests: x86: ksft_exit_pass() does not return
      selftests: kselftest: Make ksft_exit functions return void instead of int

 tools/testing/selftests/clone3/clone3_clear_sighand.c        |  2 +-
 tools/testing/selftests/clone3/clone3_set_tid.c              |  4 +++-
 tools/testing/selftests/ipc/msgque.c                         | 11 +++++------
 tools/testing/selftests/kselftest.h                          | 12 ++++++------
 .../selftests/membarrier/membarrier_test_multi_thread.c      |  2 +-
 .../selftests/membarrier/membarrier_test_single_thread.c     |  2 +-
 tools/testing/selftests/mm/compaction_test.c                 |  6 +++---
 tools/testing/selftests/mm/cow.c                             |  2 +-
 tools/testing/selftests/mm/gup_longterm.c                    |  2 +-
 tools/testing/selftests/mm/gup_test.c                        |  4 ++--
 tools/testing/selftests/mm/ksm_functional_tests.c            |  2 +-
 tools/testing/selftests/mm/madv_populate.c                   |  2 +-
 tools/testing/selftests/mm/mkdirty.c                         |  2 +-
 tools/testing/selftests/mm/pagemap_ioctl.c                   |  4 ++--
 tools/testing/selftests/mm/soft-dirty.c                      |  2 +-
 tools/testing/selftests/pidfd/pidfd_fdinfo_test.c            |  2 +-
 tools/testing/selftests/pidfd/pidfd_open_test.c              |  4 +++-
 tools/testing/selftests/pidfd/pidfd_poll_test.c              |  2 +-
 tools/testing/selftests/pidfd/pidfd_test.c                   |  2 +-
 tools/testing/selftests/resctrl/resctrl_tests.c              |  6 +++---
 tools/testing/selftests/sync/sync_test.c                     |  3 +--
 tools/testing/selftests/timers/adjtick.c                     |  4 ++--
 tools/testing/selftests/timers/alarmtimer-suspend.c          |  4 ++--
 tools/testing/selftests/timers/change_skew.c                 |  4 ++--
 tools/testing/selftests/timers/freq-step.c                   |  4 ++--
 tools/testing/selftests/timers/leap-a-day.c                  | 10 +++++-----
 tools/testing/selftests/timers/leapcrash.c                   |  4 ++--
 tools/testing/selftests/timers/mqueue-lat.c                  |  4 ++--
 tools/testing/selftests/timers/posix_timers.c                | 12 ++++++------
 tools/testing/selftests/timers/raw_skew.c                    |  6 +++---
 tools/testing/selftests/timers/set-2038.c                    |  4 ++--
 tools/testing/selftests/timers/set-tai.c                     |  4 ++--
 tools/testing/selftests/timers/set-timer-lat.c               |  4 ++--
 tools/testing/selftests/timers/set-tz.c                      |  4 ++--
 tools/testing/selftests/timers/skew_consistency.c            |  4 ++--
 tools/testing/selftests/timers/threadtest.c                  |  2 +-
 tools/testing/selftests/timers/valid-adjtimex.c              |  6 +++---
 tools/testing/selftests/x86/lam.c                            |  2 +-
 38 files changed, 81 insertions(+), 79 deletions(-)
---
base-commit: 0e275f65f3ef9c662b678655c70aca555fbde304
change-id: 20240416-ksft-exit-int-to-void-5aa9db381379

Best regards,
-- 
Nathan Chancellor <nathan@kernel.org>

[PATCH v2 04/10] selftests/mm: ksft_exit functions do not return

[Nathan Chancellor]

After commit f7d5bcd35d42 ("selftests: kselftest: Mark functions that
unconditionally call exit() as __noreturn"), ksft_exit_...() functions
are marked as __noreturn, which means the return type should not be
'int' but 'void' because they are not returning anything (and never were
since exit() has always been called).

To facilitate updating the return type of these functions, remove
'return' before the calls to ksft_exit_...(), as __noreturn prevents the
compiler from warning that a caller of the ksft_exit functions does not
return a value because the program will terminate upon calling these
functions.

Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
Cc: akpm@linux-foundation.org
Cc: linux-mm@kvack.org
---
 tools/testing/selftests/mm/compaction_test.c      | 6 +++---
 tools/testing/selftests/mm/cow.c                  | 2 +-
 tools/testing/selftests/mm/gup_longterm.c         | 2 +-
 tools/testing/selftests/mm/gup_test.c             | 4 ++--
 tools/testing/selftests/mm/ksm_functional_tests.c | 2 +-
 tools/testing/selftests/mm/madv_populate.c        | 2 +-
 tools/testing/selftests/mm/mkdirty.c              | 2 +-
 tools/testing/selftests/mm/pagemap_ioctl.c        | 4 ++--
 tools/testing/selftests/mm/soft-dirty.c           | 2 +-
 9 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/mm/compaction_test.c b/tools/testing/selftests/mm/compaction_test.c
index 533999b6c284..4f42eb7d7636 100644
--- a/tools/testing/selftests/mm/compaction_test.c
+++ b/tools/testing/selftests/mm/compaction_test.c
@@ -177,7 +177,7 @@ int main(int argc, char **argv)
 	ksft_print_header();
 
 	if (prereq() || geteuid())
-		return ksft_exit_skip("Prerequisites unsatisfied\n");
+		ksft_exit_skip("Prerequisites unsatisfied\n");
 
 	ksft_set_plan(1);
 
@@ -225,7 +225,7 @@ int main(int argc, char **argv)
 	}
 
 	if (check_compaction(mem_free, hugepage_size) == 0)
-		return ksft_exit_pass();
+		ksft_exit_pass();
 
-	return ksft_exit_fail();
+	ksft_exit_fail();
 }
diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c
index 363bf5f801be..fe078d6e1806 100644
--- a/tools/testing/selftests/mm/cow.c
+++ b/tools/testing/selftests/mm/cow.c
@@ -1779,5 +1779,5 @@ int main(int argc, char **argv)
 	if (err)
 		ksft_exit_fail_msg("%d out of %d tests failed\n",
 				   err, ksft_test_num());
-	return ksft_exit_pass();
+	ksft_exit_pass();
 }
diff --git a/tools/testing/selftests/mm/gup_longterm.c b/tools/testing/selftests/mm/gup_longterm.c
index ad168d35b23b..d7eaca5bbe9b 100644
--- a/tools/testing/selftests/mm/gup_longterm.c
+++ b/tools/testing/selftests/mm/gup_longterm.c
@@ -456,5 +456,5 @@ int main(int argc, char **argv)
 	if (err)
 		ksft_exit_fail_msg("%d out of %d tests failed\n",
 				   err, ksft_test_num());
-	return ksft_exit_pass();
+	ksft_exit_pass();
 }
diff --git a/tools/testing/selftests/mm/gup_test.c b/tools/testing/selftests/mm/gup_test.c
index 18a49c70d4c6..bd335cf9bc0e 100644
--- a/tools/testing/selftests/mm/gup_test.c
+++ b/tools/testing/selftests/mm/gup_test.c
@@ -228,7 +228,7 @@ int main(int argc, char **argv)
 			break;
 		}
 		ksft_test_result_skip("Please run this test as root\n");
-		return ksft_exit_pass();
+		ksft_exit_pass();
 	}
 
 	p = mmap(NULL, size, PROT_READ | PROT_WRITE, flags, filed, 0);
@@ -267,5 +267,5 @@ int main(int argc, char **argv)
 
 	free(tid);
 
-	return ksft_exit_pass();
+	ksft_exit_pass();
 }
diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
index d615767e396b..508287560c45 100644
--- a/tools/testing/selftests/mm/ksm_functional_tests.c
+++ b/tools/testing/selftests/mm/ksm_functional_tests.c
@@ -646,5 +646,5 @@ int main(int argc, char **argv)
 	if (err)
 		ksft_exit_fail_msg("%d out of %d tests failed\n",
 				   err, ksft_test_num());
-	return ksft_exit_pass();
+	ksft_exit_pass();
 }
diff --git a/tools/testing/selftests/mm/madv_populate.c b/tools/testing/selftests/mm/madv_populate.c
index 17bcb07f19f3..ef7d911da13e 100644
--- a/tools/testing/selftests/mm/madv_populate.c
+++ b/tools/testing/selftests/mm/madv_populate.c
@@ -307,5 +307,5 @@ int main(int argc, char **argv)
 	if (err)
 		ksft_exit_fail_msg("%d out of %d tests failed\n",
 				   err, ksft_test_num());
-	return ksft_exit_pass();
+	ksft_exit_pass();
 }
diff --git a/tools/testing/selftests/mm/mkdirty.c b/tools/testing/selftests/mm/mkdirty.c
index 301abb99e027..b8a7efe9204e 100644
--- a/tools/testing/selftests/mm/mkdirty.c
+++ b/tools/testing/selftests/mm/mkdirty.c
@@ -375,5 +375,5 @@ int main(void)
 	if (err)
 		ksft_exit_fail_msg("%d out of %d tests failed\n",
 				   err, ksft_test_num());
-	return ksft_exit_pass();
+	ksft_exit_pass();
 }
diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c b/tools/testing/selftests/mm/pagemap_ioctl.c
index d59517ed3d48..2d785aca72a5 100644
--- a/tools/testing/selftests/mm/pagemap_ioctl.c
+++ b/tools/testing/selftests/mm/pagemap_ioctl.c
@@ -1484,7 +1484,7 @@ int main(int argc, char *argv[])
 	ksft_print_header();
 
 	if (init_uffd())
-		return ksft_exit_pass();
+		ksft_exit_pass();
 
 	ksft_set_plan(115);
 
@@ -1660,5 +1660,5 @@ int main(int argc, char *argv[])
 	userfaultfd_tests();
 
 	close(pagemap_fd);
-	return ksft_exit_pass();
+	ksft_exit_pass();
 }
diff --git a/tools/testing/selftests/mm/soft-dirty.c b/tools/testing/selftests/mm/soft-dirty.c
index 7dbfa53d93a0..d9dbf879748b 100644
--- a/tools/testing/selftests/mm/soft-dirty.c
+++ b/tools/testing/selftests/mm/soft-dirty.c
@@ -209,5 +209,5 @@ int main(int argc, char **argv)
 
 	close(pagemap_fd);
 
-	return ksft_exit_pass();
+	ksft_exit_pass();
 }

-- 
2.44.0

Re: [PATCH v2 00/10] selftests: Make ksft_exit functions return void instead of int

[Shuah Khan]

On 4/24/24 11:24, Nathan Chancellor wrote:
> Hi all,
> 
> Commit f7d5bcd35d42 ("selftests: kselftest: Mark functions that
> unconditionally call exit() as __noreturn") marked functions that call
> exit() as __noreturn but it did not change the return type of these
> functions from 'void' to 'int' like it should have (since a noreturn
> function by definition cannot return an integer because it does not
> return...) because there are many tests that return the result of the
> ksft_exit function, even though it has never been used due to calling
> exit().
> 
> Prior to adding __noreturn, the compiler would not know that the functions
> that call exit() will not return, so code like
> 
>    void ksft_exit_fail(void)
>    {
>      exit(1);
>    }
> 
>    void ksft_exit_pass(void)
>    {
>      exit(0);
>    }
> 
>    int main(void)
>    {
>      int ret;
> 
>      ret = foo();
>      if (ret)
>        ksft_exit_fail();
>      ksft_exit_pass();
>    }
> 
> would cause the compiler to complain that main() does not return an
> integer, even though when ksft_exit_pass() is called, exit() will cause
> the program to terminate. So ksft_exit_...() returns int to make the
> compiler happy.
> 
>    int ksft_exit_fail(void)
>    {
>      exit(1);
>    }
> 
>    int ksft_exit_pass(void)
>    {
>      exit(0);
>    }
> 
>    int main(void)
>    {
>      int ret;
> 
>      ret = foo();
>      if (ret)
>        return ksft_exit_fail();
>      return ksft_exit_pass();
>    }
> 
> While this results in no warnings, it is weird semantically and it has
> issues as noted in the aforementioned __noreturn change. Now that
> __noreturn has been added to these functions, it is much cleaner to
> change the functions to 'void' and eliminate the return statements, as
> it has been made clear to the compiler that these functions terminate
> the program. Drop the return before all instances of ksft_exit_...() in
> a mostly mechanical way.
> 
> ---
> Changes in v2:
> - Split series into individual patches per subsystem at Shuah's
>    request and CC maintainers for subsystems that have one.
> - Rewrite commit messages for new patches and move previous commit
>    message into cover letter to high level explain all changes.
> - Carry forward Thomas and Muhammad's review on patch split, as there
>    were no functional changes, please holler if this was inappropriate.

Thank you for carrying the reviewed by tags.

> - Link to v1: https://lore.kernel.org/r/20240417-ksft-exit-int-to-void-v1-1-eff48fdbab39@kernel.org
> 
> ---
> Nathan Chancellor (10):
>        selftests/clone3: ksft_exit functions do not return
>        selftests/ipc: ksft_exit functions do not return
>        selftests: membarrier: ksft_exit_pass() does not return
>        selftests/mm: ksft_exit functions do not return
>        selftests: pidfd: ksft_exit functions do not return
>        selftests/resctrl: ksft_exit_skip() does not return
>        selftests: sync: ksft_exit_pass() does not return
>        selftests: timers: ksft_exit functions do not return
>        selftests: x86: ksft_exit_pass() does not return
>        selftests: kselftest: Make ksft_exit functions return void instead of int
> 

Applied to linux-kselftest next for Linux 6.10-rc1.

thanks,
-- Shuah