On Fri, Nov 5, 2021 at 1:45 PM Arnd Bergmann arnd@kernel.org wrote:
On Fri, Nov 5, 2021 at 9:35 PM Nick Desaulniers ndesaulniers@google.com wrote:
On Fri, Nov 5, 2021 at 9:31 AM Anders Roxell anders.roxell@linaro.org wrote:
When building selftests/timens with clang, the compiler warn about the function abs() see below:
exec.c:33:8: error: absolute value function 'abs' given an argument of type 'long' but has parameter of type 'int' which may cause truncation of value [-Werror,-Wabsolute-value] if (abs(tst.tv_sec - now.tv_sec) > 5) ^ exec.c:33:8: note: use function 'labs' instead if (abs(tst.tv_sec - now.tv_sec) > 5) ^~~ labs
Careful.
Isn't the tv_sec member of `struct timespec` a `time_t` which is 32b on 32b hosts and 64b on 64b hosts? If I'm recalling that correctly, then this patch results in a harmless (though unnecessary) sign extension for 32b targets. That should be fine, but someone like Arnd should triple check if my concern is valid or not.
It could actually be 'int', 'long' or 'long long' depending on the architecture and C library. Maybe we need a temporary variable of type 'long long' to hold the difference, and pass that to llabs()?
Yeah, that SGTM. Thanks for the review!