| Message ID | 20211105163403.3330950-1-anders.roxell@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | selftests: timers: use 'llabs()' over 'abs()' | expand |
On Fri, Nov 5, 2021 at 9:34 AM Anders Roxell <anders.roxell@linaro.org> wrote: > > When building selftests/timers with clang, the compiler warn about the > function abs() see below: > > posix_timers.c:69:6: warning: absolute value function 'abs' given an argument of type 'long long' but has parameter of type 'int' which may cause truncation of value [-Wabsolute-value] > if (abs(diff - DELAY * USECS_PER_SEC) > USECS_PER_SEC / 2) { > ^ > posix_timers.c:69:6: note: use function 'llabs' instead > if (abs(diff - DELAY * USECS_PER_SEC) > USECS_PER_SEC / 2) { > ^~~ > llabs > > The note indicates what to do, Rework to use the function 'llabs()'. > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> Thanks for the patch! Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> I wonder why tools/testing/selftests/timers/adjtick.c redefines llabs when it already includes stdlib.h, and how that doesn't result in some kind of compiler diagnostic. > --- > tools/testing/selftests/timers/posix_timers.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/timers/posix_timers.c b/tools/testing/selftests/timers/posix_timers.c > index 337424c5c987..73fb27901a1d 100644 > --- a/tools/testing/selftests/timers/posix_timers.c > +++ b/tools/testing/selftests/timers/posix_timers.c > @@ -66,7 +66,7 @@ static int check_diff(struct timeval start, struct timeval end) > diff = end.tv_usec - start.tv_usec; > diff += (end.tv_sec - start.tv_sec) * USECS_PER_SEC; > > - if (abs(diff - DELAY * USECS_PER_SEC) > USECS_PER_SEC / 2) { > + if (llabs(diff - DELAY * USECS_PER_SEC) > USECS_PER_SEC / 2) { > printf("Diff too high: %lld..", diff); > return -1; > } > -- > 2.33.0 >
On 11/5/21 2:44 PM, Nick Desaulniers wrote: > On Fri, Nov 5, 2021 at 9:34 AM Anders Roxell <anders.roxell@linaro.org> wrote: >> >> When building selftests/timers with clang, the compiler warn about the >> function abs() see below: >> >> posix_timers.c:69:6: warning: absolute value function 'abs' given an argument of type 'long long' but has parameter of type 'int' which may cause truncation of value [-Wabsolute-value] >> if (abs(diff - DELAY * USECS_PER_SEC) > USECS_PER_SEC / 2) { >> ^ >> posix_timers.c:69:6: note: use function 'llabs' instead >> if (abs(diff - DELAY * USECS_PER_SEC) > USECS_PER_SEC / 2) { >> ^~~ >> llabs >> >> The note indicates what to do, Rework to use the function 'llabs()'. >> >> Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > > Thanks for the patch! > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > I wonder why tools/testing/selftests/timers/adjtick.c redefines llabs > when it already includes stdlib.h, and how that doesn't result in some > kind of compiler diagnostic. > Hmm. The define in /usr/include/stdlib.h is under #ifdef __USE_ISOC99 Maybe be that explains the reason for the definitions in the other two timers file? Anders, I would like to understand this before I take this patch. I see you sent several patches to other tests as well. Would be useful at least understand why these defines exist. thanks, -- Shuah
diff --git a/tools/testing/selftests/timers/posix_timers.c b/tools/testing/selftests/timers/posix_timers.c index 337424c5c987..73fb27901a1d 100644 --- a/tools/testing/selftests/timers/posix_timers.c +++ b/tools/testing/selftests/timers/posix_timers.c @@ -66,7 +66,7 @@ static int check_diff(struct timeval start, struct timeval end) diff = end.tv_usec - start.tv_usec; diff += (end.tv_sec - start.tv_sec) * USECS_PER_SEC; - if (abs(diff - DELAY * USECS_PER_SEC) > USECS_PER_SEC / 2) { + if (llabs(diff - DELAY * USECS_PER_SEC) > USECS_PER_SEC / 2) { printf("Diff too high: %lld..", diff); return -1; }
When building selftests/timers with clang, the compiler warn about the function abs() see below: posix_timers.c:69:6: warning: absolute value function 'abs' given an argument of type 'long long' but has parameter of type 'int' which may cause truncation of value [-Wabsolute-value] if (abs(diff - DELAY * USECS_PER_SEC) > USECS_PER_SEC / 2) { ^ posix_timers.c:69:6: note: use function 'llabs' instead if (abs(diff - DELAY * USECS_PER_SEC) > USECS_PER_SEC / 2) { ^~~ llabs The note indicates what to do, Rework to use the function 'llabs()'. Signed-off-by: Anders Roxell <anders.roxell@linaro.org> --- tools/testing/selftests/timers/posix_timers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)