So, the struct adjtimex freq field takes a signed value who's units are in shifted (<<16) parts-per-million.
Unfortunately for negative adjustments, the straightforward use of: freq = ppm<<16 will trip undefined behavior warnings with clang:
valid-adjtimex.c:66:6: warning: shifting a negative signed value is undefined [-Wshift-negative-value] -499<<16, ~~~~^ valid-adjtimex.c:67:6: warning: shifting a negative signed value is undefined [-Wshift-negative-value] -450<<16, ~~~~^ ...
So fix our use of shifting negative values in the valid-adjtimex test case to use multiply by (1<<16) to avoid this.
The patch also aligns the values a bit to make it look nicer.
Cc: Thomas Gleixner tglx@linutronix.de Cc: Stephen Boyd sboyd@kernel.org Cc: Anna-Maria Behnsen anna-maria@linutronix.de Cc: Frederic Weisbecker frederic@kernel.org Cc: Shuah Khan shuah@kernel.org Cc: Nathan Chancellor nathan@kernel.org Cc: Nick Desaulniers ndesaulniers@google.com Cc: Lee Jones joneslee@google.com Cc: Muhammad Usama Anjum usama.anjum@collabora.com Cc: linux-kselftest@vger.kernel.org Reported-by: Lee Jones joneslee@google.com Reported-by: Muhammad Usama Anjum usama.anjum@collabora.com Link: https://lore.kernel.org/lkml/0c6d4f0d-2064-4444-986b-1d1ed782135f@collabora.... Signed-off-by: John Stultz jstultz@google.com --- .../testing/selftests/timers/valid-adjtimex.c | 69 ++++++++++--------- 1 file changed, 35 insertions(+), 34 deletions(-)
diff --git a/tools/testing/selftests/timers/valid-adjtimex.c b/tools/testing/selftests/timers/valid-adjtimex.c index 48b9a803235a..9606d45767e7 100644 --- a/tools/testing/selftests/timers/valid-adjtimex.c +++ b/tools/testing/selftests/timers/valid-adjtimex.c @@ -62,45 +62,46 @@ int clear_time_state(void) #define NUM_FREQ_OUTOFRANGE 4 #define NUM_FREQ_INVALID 2
+#define SHIFTED_PPM (1 << 16) long valid_freq[NUM_FREQ_VALID] = { - -499<<16, - -450<<16, - -400<<16, - -350<<16, - -300<<16, - -250<<16, - -200<<16, - -150<<16, - -100<<16, - -75<<16, - -50<<16, - -25<<16, - -10<<16, - -5<<16, - -1<<16, + -499 * SHIFTED_PPM, + -450 * SHIFTED_PPM, + -400 * SHIFTED_PPM, + -350 * SHIFTED_PPM, + -300 * SHIFTED_PPM, + -250 * SHIFTED_PPM, + -200 * SHIFTED_PPM, + -150 * SHIFTED_PPM, + -100 * SHIFTED_PPM, + -75 * SHIFTED_PPM, + -50 * SHIFTED_PPM, + -25 * SHIFTED_PPM, + -10 * SHIFTED_PPM, + -5 * SHIFTED_PPM, + -1 * SHIFTED_PPM, -1000, - 1<<16, - 5<<16, - 10<<16, - 25<<16, - 50<<16, - 75<<16, - 100<<16, - 150<<16, - 200<<16, - 250<<16, - 300<<16, - 350<<16, - 400<<16, - 450<<16, - 499<<16, + 1 * SHIFTED_PPM, + 5 * SHIFTED_PPM, + 10 * SHIFTED_PPM, + 25 * SHIFTED_PPM, + 50 * SHIFTED_PPM, + 75 * SHIFTED_PPM, + 100 * SHIFTED_PPM, + 150 * SHIFTED_PPM, + 200 * SHIFTED_PPM, + 250 * SHIFTED_PPM, + 300 * SHIFTED_PPM, + 350 * SHIFTED_PPM, + 400 * SHIFTED_PPM, + 450 * SHIFTED_PPM, + 499 * SHIFTED_PPM, };
long outofrange_freq[NUM_FREQ_OUTOFRANGE] = { - -1000<<16, - -550<<16, - 550<<16, - 1000<<16, + -1000 * SHIFTED_PPM, + -550 * SHIFTED_PPM, + 550 * SHIFTED_PPM, + 1000 * SHIFTED_PPM, };
#define LONG_MAX (~0UL>>1)
On 4/10/24 1:22 AM, John Stultz wrote:
So, the struct adjtimex freq field takes a signed value who's units are in shifted (<<16) parts-per-million.
Unfortunately for negative adjustments, the straightforward use of: freq = ppm<<16 will trip undefined behavior warnings with clang:
valid-adjtimex.c:66:6: warning: shifting a negative signed value is undefined [-Wshift-negative-value] -499<<16, ~~~~^ valid-adjtimex.c:67:6: warning: shifting a negative signed value is undefined [-Wshift-negative-value] -450<<16, ~~~~^ ...
So fix our use of shifting negative values in the valid-adjtimex test case to use multiply by (1<<16) to avoid this.
The patch also aligns the values a bit to make it look nicer.
Cc: Thomas Gleixner tglx@linutronix.de Cc: Stephen Boyd sboyd@kernel.org Cc: Anna-Maria Behnsen anna-maria@linutronix.de Cc: Frederic Weisbecker frederic@kernel.org Cc: Shuah Khan shuah@kernel.org Cc: Nathan Chancellor nathan@kernel.org Cc: Nick Desaulniers ndesaulniers@google.com Cc: Lee Jones joneslee@google.com Cc: Muhammad Usama Anjum usama.anjum@collabora.com Cc: linux-kselftest@vger.kernel.org Reported-by: Lee Jones joneslee@google.com Reported-by: Muhammad Usama Anjum usama.anjum@collabora.com Link: https://lore.kernel.org/lkml/0c6d4f0d-2064-4444-986b-1d1ed782135f@collabora.... Signed-off-by: John Stultz jstultz@google.com
Reviewed-by: Muhammad Usama Anjum usama.anjum@collabora.com
.../testing/selftests/timers/valid-adjtimex.c | 69 ++++++++++--------- 1 file changed, 35 insertions(+), 34 deletions(-)
diff --git a/tools/testing/selftests/timers/valid-adjtimex.c b/tools/testing/selftests/timers/valid-adjtimex.c index 48b9a803235a..9606d45767e7 100644 --- a/tools/testing/selftests/timers/valid-adjtimex.c +++ b/tools/testing/selftests/timers/valid-adjtimex.c @@ -62,45 +62,46 @@ int clear_time_state(void) #define NUM_FREQ_OUTOFRANGE 4 #define NUM_FREQ_INVALID 2 +#define SHIFTED_PPM (1 << 16) long valid_freq[NUM_FREQ_VALID] = {
- -499<<16,
- -450<<16,
- -400<<16,
- -350<<16,
- -300<<16,
- -250<<16,
- -200<<16,
- -150<<16,
- -100<<16,
- -75<<16,
- -50<<16,
- -25<<16,
- -10<<16,
- -5<<16,
- -1<<16,
-499 * SHIFTED_PPM,
-450 * SHIFTED_PPM,
-400 * SHIFTED_PPM,
-350 * SHIFTED_PPM,
-300 * SHIFTED_PPM,
-250 * SHIFTED_PPM,
-200 * SHIFTED_PPM,
-150 * SHIFTED_PPM,
-100 * SHIFTED_PPM,
-75 * SHIFTED_PPM,
-50 * SHIFTED_PPM,
-25 * SHIFTED_PPM,
-10 * SHIFTED_PPM,
-5 * SHIFTED_PPM,
-1000,-1 * SHIFTED_PPM,
- 1<<16,
- 5<<16,
- 10<<16,
- 25<<16,
- 50<<16,
- 75<<16,
- 100<<16,
- 150<<16,
- 200<<16,
- 250<<16,
- 300<<16,
- 350<<16,
- 400<<16,
- 450<<16,
- 499<<16,
1 * SHIFTED_PPM,
5 * SHIFTED_PPM,
10 * SHIFTED_PPM,
25 * SHIFTED_PPM,
50 * SHIFTED_PPM,
75 * SHIFTED_PPM,
100 * SHIFTED_PPM,
150 * SHIFTED_PPM,
200 * SHIFTED_PPM,
250 * SHIFTED_PPM,
300 * SHIFTED_PPM,
350 * SHIFTED_PPM,
400 * SHIFTED_PPM,
450 * SHIFTED_PPM,
499 * SHIFTED_PPM,
}; long outofrange_freq[NUM_FREQ_OUTOFRANGE] = {
- -1000<<16,
- -550<<16,
- 550<<16,
- 1000<<16,
- -1000 * SHIFTED_PPM,
-550 * SHIFTED_PPM,
550 * SHIFTED_PPM,
1000 * SHIFTED_PPM,
}; #define LONG_MAX (~0UL>>1)
On 4/9/24 14:22, John Stultz wrote:
So, the struct adjtimex freq field takes a signed value who's units are in shifted (<<16) parts-per-million.
Unfortunately for negative adjustments, the straightforward use of: freq = ppm<<16 will trip undefined behavior warnings with clang:
valid-adjtimex.c:66:6: warning: shifting a negative signed value is undefined [-Wshift-negative-value] -499<<16, ~~~~^ valid-adjtimex.c:67:6: warning: shifting a negative signed value is undefined [-Wshift-negative-value] -450<<16, ~~~~^ ...
So fix our use of shifting negative values in the valid-adjtimex test case to use multiply by (1<<16) to avoid this.
The patch also aligns the values a bit to make it look nicer.
Cc: Thomas Gleixner tglx@linutronix.de Cc: Stephen Boyd sboyd@kernel.org Cc: Anna-Maria Behnsen anna-maria@linutronix.de Cc: Frederic Weisbecker frederic@kernel.org Cc: Shuah Khan shuah@kernel.org Cc: Nathan Chancellor nathan@kernel.org Cc: Nick Desaulniers ndesaulniers@google.com Cc: Lee Jones joneslee@google.com Cc: Muhammad Usama Anjum usama.anjum@collabora.com Cc: linux-kselftest@vger.kernel.org Reported-by: Lee Jones joneslee@google.com Reported-by: Muhammad Usama Anjum usama.anjum@collabora.com Link: https://lore.kernel.org/lkml/0c6d4f0d-2064-4444-986b-1d1ed782135f@collabora.... Signed-off-by: John Stultz jstultz@google.com
Applied to linux-kselftest next for Linux6.10-rc1.
thanks, -- Shuah
linux-kselftest-mirror@lists.linaro.org