| Message ID | 20240424-ksft-exit-int-to-void-v2-0-c35f3b8c9ca0@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | selftests: Make ksft_exit functions return void instead of int | expand |
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
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,