Re: [chrony-dev] [PATCH] macOS 11.0 (Big Sur) beta - Bug in implementation of ntp_adjtime()

[Miroslav Lichvar]

On Sun, Aug 30, 2020 at 09:49:22AM +1200, Bryan Christianson wrote:
> macOS 11.0 (Big Sur) beta incorrectly returns timex.freq as an unsigned 
> number. This patch is a workaround for the bug and should be removed when 
> Apple fix the problem (assuming they will). The patch also extends the 
> ntpadjtime test to cover negative frequencies.
> ---
>  sys_timex.c  | 23 +--
>  test/kernel/ntpadjtime.c |  2 +-

Can you please split it into two patches, e.g. "test: extend
frequency in ntp_adjtime() test", and "sys_timex: add workaround for
broken ntp_adjtime() on macOS"?

> +static inline double
> +txc_freq(const struct timex *txc)

No need for the inline keyword (it's ok to rely on the compiler to
inline automatically when appropriate) and please find a bit more
descriptive name for the function, e.g. "convert_timex_frequency".

Thanks,

-- 
Miroslav Lichvar


-- 
To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" 
in the subject.
For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the 
subject.
Trouble?  Email listmas...@chrony.tuxfamily.org.

[chrony-dev] [PATCH] macOS 11.0 (Big Sur) beta - Bug in implementation of ntp_adjtime()

[Bryan Christianson]

macOS 11.0 (Big Sur) beta incorrectly returns timex.freq as an unsigned number. 
This patch is a workaround for the bug and should be removed when Apple fix the 
problem (assuming they will). The patch also extends the ntpadjtime test to 
cover negative frequencies.
---
 sys_timex.c  | 23 +--
 test/kernel/ntpadjtime.c |  2 +-
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/sys_timex.c b/sys_timex.c
index 0a6b438..aa1508a 100644
--- a/sys_timex.c
+++ b/sys_timex.c
@@ -68,6 +68,25 @@ static int sys_tai_offset;
 
 /* == */
 
+static inline double
+txc_freq(const struct timex *txc)
+{
+  double freq_ppm;
+
+  freq_ppm = txc->freq / FREQ_SCALE;
+
+#ifdef MACOSX
+  /* temporary work-around for Apple bug treating freq as unsigned number */
+  if (freq_ppm > 32767) {
+freq_ppm -= 65536;
+  }
+#endif
+
+  return -freq_ppm;
+}
+
+/* == */
+
 static double
 read_frequency(void)
 {
@@ -77,7 +96,7 @@ read_frequency(void)
 
   SYS_Timex_Adjust(, 0);
 
-  return txc.freq / -FREQ_SCALE;
+  return txc_freq();
 }
 
 /* == */
@@ -92,7 +111,7 @@ set_frequency(double freq_ppm)
 
   SYS_Timex_Adjust(, 0);
 
-  return txc.freq / -FREQ_SCALE;
+  return txc_freq();
 }
 
 /* == */
diff --git a/test/kernel/ntpadjtime.c b/test/kernel/ntpadjtime.c
index d6be154..96c069f 100644
--- a/test/kernel/ntpadjtime.c
+++ b/test/kernel/ntpadjtime.c
@@ -52,7 +52,7 @@ test_freqrange(void)
 
   printf("freq range:\n");
 
-  for (i = 0; i <= 1000; i += 50) {
+  for (i = -1000; i <= 1000; i += 50) {
 t.modes = MOD_FREQUENCY;
 t.freq = i << 16;
 printf("%4d ppm => ", i);
-- 
2.24.3 (Apple Git-128)


-- 
To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" 
in the subject.
For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the 
subject.
Trouble?  Email listmas...@chrony.tuxfamily.org.