| Message ID | 20240503032956.89290-1-jhubbard@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 35a9e53bb1699af1cd531d43ac420e21a88c27c2 |
| Headers | show |
| Series | selftest/timerns: fix clang build failures for abs() calls | expand |
On Fri, May 3, 2024 at 4:30 AM John Hubbard <jhubbard@nvidia.com> wrote: > > First of all, in order to build with clang at all, one must first apply > Valentin Obst's build fix for LLVM [1]. Once that is done, then when > building with clang, via: > > make LLVM=1 -C tools/testing/selftests > > ...then clang warns about mismatches between the expected and required > integer length being supplied to abs(3). > > Fix this by using the correct variant of abs(3): labs(3) or llabs(3), in > these cases. > > [1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c49f@valentinobst.de/ > > Signed-off-by: John Hubbard <jhubbard@nvidia.com> LGTM, even potentially fixes the testing post-2038. Reviewed-by: Dmitry Safonov <dima@arista.com> > --- > tools/testing/selftests/timens/exec.c | 6 +++--- > tools/testing/selftests/timens/timer.c | 2 +- > tools/testing/selftests/timens/timerfd.c | 2 +- > tools/testing/selftests/timens/vfork_exec.c | 4 ++-- > 4 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/tools/testing/selftests/timens/exec.c b/tools/testing/selftests/timens/exec.c > index e40dc5be2f66..d12ff955de0d 100644 > --- a/tools/testing/selftests/timens/exec.c > +++ b/tools/testing/selftests/timens/exec.c > @@ -30,7 +30,7 @@ int main(int argc, char *argv[]) > > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, &tst, i); > - if (abs(tst.tv_sec - now.tv_sec) > 5) > + if (labs(tst.tv_sec - now.tv_sec) > 5) > return pr_fail("%ld %ld\n", now.tv_sec, tst.tv_sec); > } > return 0; > @@ -50,7 +50,7 @@ int main(int argc, char *argv[]) > > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, &tst, i); > - if (abs(tst.tv_sec - now.tv_sec) > 5) > + if (labs(tst.tv_sec - now.tv_sec) > 5) > return pr_fail("%ld %ld\n", > now.tv_sec, tst.tv_sec); > } > @@ -70,7 +70,7 @@ int main(int argc, char *argv[]) > /* Check that a child process is in the new timens. */ > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, &tst, i); > - if (abs(tst.tv_sec - now.tv_sec - OFFSET) > 5) > + if (labs(tst.tv_sec - now.tv_sec - OFFSET) > 5) > return pr_fail("%ld %ld\n", > now.tv_sec + OFFSET, tst.tv_sec); > } > diff --git a/tools/testing/selftests/timens/timer.c b/tools/testing/selftests/timens/timer.c > index 5e7f0051bd7b..5b939f59dfa4 100644 > --- a/tools/testing/selftests/timens/timer.c > +++ b/tools/testing/selftests/timens/timer.c > @@ -56,7 +56,7 @@ int run_test(int clockid, struct timespec now) > return pr_perror("timerfd_gettime"); > > elapsed = new_value.it_value.tv_sec; > - if (abs(elapsed - 3600) > 60) { > + if (llabs(elapsed - 3600) > 60) { > ksft_test_result_fail("clockid: %d elapsed: %lld\n", > clockid, elapsed); > return 1; > diff --git a/tools/testing/selftests/timens/timerfd.c b/tools/testing/selftests/timens/timerfd.c > index 9edd43d6b2c1..a4196bbd6e33 100644 > --- a/tools/testing/selftests/timens/timerfd.c > +++ b/tools/testing/selftests/timens/timerfd.c > @@ -61,7 +61,7 @@ int run_test(int clockid, struct timespec now) > return pr_perror("timerfd_gettime(%d)", clockid); > > elapsed = new_value.it_value.tv_sec; > - if (abs(elapsed - 3600) > 60) { > + if (llabs(elapsed - 3600) > 60) { > ksft_test_result_fail("clockid: %d elapsed: %lld\n", > clockid, elapsed); > return 1; > diff --git a/tools/testing/selftests/timens/vfork_exec.c b/tools/testing/selftests/timens/vfork_exec.c > index beb7614941fb..5b8907bf451d 100644 > --- a/tools/testing/selftests/timens/vfork_exec.c > +++ b/tools/testing/selftests/timens/vfork_exec.c > @@ -32,7 +32,7 @@ static void *tcheck(void *_args) > > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, &tst, i); > - if (abs(tst.tv_sec - now->tv_sec) > 5) { > + if (labs(tst.tv_sec - now->tv_sec) > 5) { > pr_fail("%s: in-thread: unexpected value: %ld (%ld)\n", > args->tst_name, tst.tv_sec, now->tv_sec); > return (void *)1UL; > @@ -64,7 +64,7 @@ static int check(char *tst_name, struct timespec *now) > > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, &tst, i); > - if (abs(tst.tv_sec - now->tv_sec) > 5) > + if (labs(tst.tv_sec - now->tv_sec) > 5) > return pr_fail("%s: unexpected value: %ld (%ld)\n", > tst_name, tst.tv_sec, now->tv_sec); > } > > base-commit: f03359bca01bf4372cf2c118cd9a987a5951b1c8 > prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27 > -- > 2.45.0 >
On 5/3/24 8:29 AM, John Hubbard wrote: > First of all, in order to build with clang at all, one must first apply > Valentin Obst's build fix for LLVM [1]. Once that is done, then when > building with clang, via: > > make LLVM=1 -C tools/testing/selftests > > ...then clang warns about mismatches between the expected and required > integer length being supplied to abs(3). > > Fix this by using the correct variant of abs(3): labs(3) or llabs(3), in > these cases. > > [1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c49f@valentinobst.de/ > > Signed-off-by: John Hubbard <jhubbard@nvidia.com> Thanks for fixing! Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > tools/testing/selftests/timens/exec.c | 6 +++--- > tools/testing/selftests/timens/timer.c | 2 +- > tools/testing/selftests/timens/timerfd.c | 2 +- > tools/testing/selftests/timens/vfork_exec.c | 4 ++-- > 4 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/tools/testing/selftests/timens/exec.c b/tools/testing/selftests/timens/exec.c > index e40dc5be2f66..d12ff955de0d 100644 > --- a/tools/testing/selftests/timens/exec.c > +++ b/tools/testing/selftests/timens/exec.c > @@ -30,7 +30,7 @@ int main(int argc, char *argv[]) > > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, &tst, i); > - if (abs(tst.tv_sec - now.tv_sec) > 5) > + if (labs(tst.tv_sec - now.tv_sec) > 5) > return pr_fail("%ld %ld\n", now.tv_sec, tst.tv_sec); > } > return 0; > @@ -50,7 +50,7 @@ int main(int argc, char *argv[]) > > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, &tst, i); > - if (abs(tst.tv_sec - now.tv_sec) > 5) > + if (labs(tst.tv_sec - now.tv_sec) > 5) > return pr_fail("%ld %ld\n", > now.tv_sec, tst.tv_sec); > } > @@ -70,7 +70,7 @@ int main(int argc, char *argv[]) > /* Check that a child process is in the new timens. */ > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, &tst, i); > - if (abs(tst.tv_sec - now.tv_sec - OFFSET) > 5) > + if (labs(tst.tv_sec - now.tv_sec - OFFSET) > 5) > return pr_fail("%ld %ld\n", > now.tv_sec + OFFSET, tst.tv_sec); > } > diff --git a/tools/testing/selftests/timens/timer.c b/tools/testing/selftests/timens/timer.c > index 5e7f0051bd7b..5b939f59dfa4 100644 > --- a/tools/testing/selftests/timens/timer.c > +++ b/tools/testing/selftests/timens/timer.c > @@ -56,7 +56,7 @@ int run_test(int clockid, struct timespec now) > return pr_perror("timerfd_gettime"); > > elapsed = new_value.it_value.tv_sec; > - if (abs(elapsed - 3600) > 60) { > + if (llabs(elapsed - 3600) > 60) { > ksft_test_result_fail("clockid: %d elapsed: %lld\n", > clockid, elapsed); > return 1; > diff --git a/tools/testing/selftests/timens/timerfd.c b/tools/testing/selftests/timens/timerfd.c > index 9edd43d6b2c1..a4196bbd6e33 100644 > --- a/tools/testing/selftests/timens/timerfd.c > +++ b/tools/testing/selftests/timens/timerfd.c > @@ -61,7 +61,7 @@ int run_test(int clockid, struct timespec now) > return pr_perror("timerfd_gettime(%d)", clockid); > > elapsed = new_value.it_value.tv_sec; > - if (abs(elapsed - 3600) > 60) { > + if (llabs(elapsed - 3600) > 60) { > ksft_test_result_fail("clockid: %d elapsed: %lld\n", > clockid, elapsed); > return 1; > diff --git a/tools/testing/selftests/timens/vfork_exec.c b/tools/testing/selftests/timens/vfork_exec.c > index beb7614941fb..5b8907bf451d 100644 > --- a/tools/testing/selftests/timens/vfork_exec.c > +++ b/tools/testing/selftests/timens/vfork_exec.c > @@ -32,7 +32,7 @@ static void *tcheck(void *_args) > > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, &tst, i); > - if (abs(tst.tv_sec - now->tv_sec) > 5) { > + if (labs(tst.tv_sec - now->tv_sec) > 5) { > pr_fail("%s: in-thread: unexpected value: %ld (%ld)\n", > args->tst_name, tst.tv_sec, now->tv_sec); > return (void *)1UL; > @@ -64,7 +64,7 @@ static int check(char *tst_name, struct timespec *now) > > for (i = 0; i < 2; i++) { > _gettime(CLOCK_MONOTONIC, &tst, i); > - if (abs(tst.tv_sec - now->tv_sec) > 5) > + if (labs(tst.tv_sec - now->tv_sec) > 5) > return pr_fail("%s: unexpected value: %ld (%ld)\n", > tst_name, tst.tv_sec, now->tv_sec); > } > > base-commit: f03359bca01bf4372cf2c118cd9a987a5951b1c8 > prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27
diff --git a/tools/testing/selftests/timens/exec.c b/tools/testing/selftests/timens/exec.c index e40dc5be2f66..d12ff955de0d 100644 --- a/tools/testing/selftests/timens/exec.c +++ b/tools/testing/selftests/timens/exec.c @@ -30,7 +30,7 @@ int main(int argc, char *argv[]) for (i = 0; i < 2; i++) { _gettime(CLOCK_MONOTONIC, &tst, i); - if (abs(tst.tv_sec - now.tv_sec) > 5) + if (labs(tst.tv_sec - now.tv_sec) > 5) return pr_fail("%ld %ld\n", now.tv_sec, tst.tv_sec); } return 0; @@ -50,7 +50,7 @@ int main(int argc, char *argv[]) for (i = 0; i < 2; i++) { _gettime(CLOCK_MONOTONIC, &tst, i); - if (abs(tst.tv_sec - now.tv_sec) > 5) + if (labs(tst.tv_sec - now.tv_sec) > 5) return pr_fail("%ld %ld\n", now.tv_sec, tst.tv_sec); } @@ -70,7 +70,7 @@ int main(int argc, char *argv[]) /* Check that a child process is in the new timens. */ for (i = 0; i < 2; i++) { _gettime(CLOCK_MONOTONIC, &tst, i); - if (abs(tst.tv_sec - now.tv_sec - OFFSET) > 5) + if (labs(tst.tv_sec - now.tv_sec - OFFSET) > 5) return pr_fail("%ld %ld\n", now.tv_sec + OFFSET, tst.tv_sec); } diff --git a/tools/testing/selftests/timens/timer.c b/tools/testing/selftests/timens/timer.c index 5e7f0051bd7b..5b939f59dfa4 100644 --- a/tools/testing/selftests/timens/timer.c +++ b/tools/testing/selftests/timens/timer.c @@ -56,7 +56,7 @@ int run_test(int clockid, struct timespec now) return pr_perror("timerfd_gettime"); elapsed = new_value.it_value.tv_sec; - if (abs(elapsed - 3600) > 60) { + if (llabs(elapsed - 3600) > 60) { ksft_test_result_fail("clockid: %d elapsed: %lld\n", clockid, elapsed); return 1; diff --git a/tools/testing/selftests/timens/timerfd.c b/tools/testing/selftests/timens/timerfd.c index 9edd43d6b2c1..a4196bbd6e33 100644 --- a/tools/testing/selftests/timens/timerfd.c +++ b/tools/testing/selftests/timens/timerfd.c @@ -61,7 +61,7 @@ int run_test(int clockid, struct timespec now) return pr_perror("timerfd_gettime(%d)", clockid); elapsed = new_value.it_value.tv_sec; - if (abs(elapsed - 3600) > 60) { + if (llabs(elapsed - 3600) > 60) { ksft_test_result_fail("clockid: %d elapsed: %lld\n", clockid, elapsed); return 1; diff --git a/tools/testing/selftests/timens/vfork_exec.c b/tools/testing/selftests/timens/vfork_exec.c index beb7614941fb..5b8907bf451d 100644 --- a/tools/testing/selftests/timens/vfork_exec.c +++ b/tools/testing/selftests/timens/vfork_exec.c @@ -32,7 +32,7 @@ static void *tcheck(void *_args) for (i = 0; i < 2; i++) { _gettime(CLOCK_MONOTONIC, &tst, i); - if (abs(tst.tv_sec - now->tv_sec) > 5) { + if (labs(tst.tv_sec - now->tv_sec) > 5) { pr_fail("%s: in-thread: unexpected value: %ld (%ld)\n", args->tst_name, tst.tv_sec, now->tv_sec); return (void *)1UL; @@ -64,7 +64,7 @@ static int check(char *tst_name, struct timespec *now) for (i = 0; i < 2; i++) { _gettime(CLOCK_MONOTONIC, &tst, i); - if (abs(tst.tv_sec - now->tv_sec) > 5) + if (labs(tst.tv_sec - now->tv_sec) > 5) return pr_fail("%s: unexpected value: %ld (%ld)\n", tst_name, tst.tv_sec, now->tv_sec); }
First of all, in order to build with clang at all, one must first apply Valentin Obst's build fix for LLVM [1]. Once that is done, then when building with clang, via: make LLVM=1 -C tools/testing/selftests ...then clang warns about mismatches between the expected and required integer length being supplied to abs(3). Fix this by using the correct variant of abs(3): labs(3) or llabs(3), in these cases. [1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c49f@valentinobst.de/ Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- tools/testing/selftests/timens/exec.c | 6 +++--- tools/testing/selftests/timens/timer.c | 2 +- tools/testing/selftests/timens/timerfd.c | 2 +- tools/testing/selftests/timens/vfork_exec.c | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) base-commit: f03359bca01bf4372cf2c118cd9a987a5951b1c8 prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27