Fix PPS offset display bug by properly thread-locking some accesses.

7 files changed, 58 insertions, 20 deletions

diff --git a/gpsd.h-tail b/gpsd.h-tail
index 441b4ec4..9dbf0d7a 100644
--- a/gpsd.h-tail
+++ b/gpsd.h-tail
@@ -483,6 +483,8 @@ struct gps_device_t {
     /*@null@*/ char *(*thread_report_hook)(struct gps_device_t *,
 					   struct timedrift_t *);
     /*@null@*/ void (*thread_wrap_hook)(struct gps_device_t *);
+    volatile struct timedrift_t ppslast;
+    volatile int ppscount;
 #endif /* PPS_ENABLE */
     double mag_var;			/* magnetic variation in degrees */
     bool back_to_nmea;			/* back to NMEA on revert? */
@@ -870,6 +872,7 @@ extern void ntpshm_link_activate(struct gps_device_t *);
 
 extern void pps_thread_activate(struct gps_device_t *);
 extern void pps_thread_deactivate(struct gps_device_t *);
+extern int pps_thread_lastpps(struct gps_device_t *, struct timedrift_t *);
 
 extern void gpsd_acquire_reporting_lock(void);
 extern void gpsd_release_reporting_lock(void);
diff --git a/gpsmon.c b/gpsmon.c
index ccdf0c50..ef7a1468 100644
--- a/gpsmon.c
+++ b/gpsmon.c
@@ -46,7 +46,6 @@ extern const struct gps_type_t nmea0183;
 struct gps_device_t session;
 WINDOW *devicewin;
 bool serial;
-double timedelta = 0;
 
 /* These are private */
 static struct gps_context_t context;
@@ -863,17 +862,7 @@ static bool do_command(void)
 
 #ifdef PPS_ENABLE
 static /*@observer@*/ char *pps_report(struct gps_device_t *session UNUSED,
-			struct timedrift_t *td) {
-    /*
-     * Ugh.  Access through a shared global is nasty.
-     * This may be a layer violation that needs to be fixed.
-     *
-     * Read access to timedelta is not thread-locked.
-     * Instead we're relying on access to floats to be atomic.
-     */
-    /*@-type@*/ /* splint is confused about struct timespec */
-    timedelta = timespec_diff_ns(td->real, td->clock) * 1e-9;
-    /*@+type@*/
+			struct timedrift_t *td UNUSED) {
     packet_log("#------------------------------------" 
 	       " PPS "
 	       "------------------------------------#\n");
diff --git a/gpsmon.h b/gpsmon.h
index cc6cd156..cb90da28 100644
--- a/gpsmon.h
+++ b/gpsmon.h
@@ -35,7 +35,6 @@ extern void monitor_complain(const char *fmt, ...);
 extern WINDOW *devicewin;
 extern struct gps_device_t	session;
 extern bool serial;	/* True - direct mode, False - daemon mode */
-extern double timedelta;
 
 #endif /* _GPSD_GPSMON_H_ */
 /* gpsmon.h ends here */
diff --git a/monitor_nmea.c b/monitor_nmea.c
index 5efa791b..4f7d706c 100644
--- a/monitor_nmea.c
+++ b/monitor_nmea.c
@@ -174,6 +174,8 @@ static void cooked_pvt(void)
 static void nmea_update(void)
 {
     char **fields;
+    struct timedrift_t drift;
+    double timedelta;
 
     assert(cookedwin != NULL);
     assert(nmeawin != NULL);
@@ -314,8 +316,12 @@ static void nmea_update(void)
 	}
     }
 
-    if (timedelta != 0)
-	(void)mvwprintw(gpgsawin, 4, 13, "%f", timedelta);
+    pps_thread_lastpps(&session, &drift);
+    /*@-type@*/ /* splint is confused about struct timespec */
+    timedelta = timespec_diff_ns(drift.real, drift.clock) * 1e-9;
+    /*@+type@*/
+    (void)mvwprintw(gpgsawin, 4, 13, "%.9f", timedelta);
+    wnoutrefresh(gpgsawin);
 }
 
 /*@ +globstate +nullpass */
diff --git a/monitor_sirf.c b/monitor_sirf.c
index 2d5e1d5f..2d49fdc8 100644
--- a/monitor_sirf.c
+++ b/monitor_sirf.c
@@ -277,6 +277,7 @@ static void sirf_update(void)
     size_t len;
     uint8_t dgps;
     char tbuf[JSON_DATE_MAX+1];
+    struct timedrift_t drift;
 
     /* splint pacification */
     assert(mid2win!=NULL && mid27win != NULL);
@@ -385,9 +386,6 @@ static void sirf_update(void)
 	display(mid7win, 1, 16, "%lu", getbeu32(buf, 8));	/* Clock drift */
 	display(mid7win, 1, 29, "%lu", getbeu32(buf, 12));	/* Clock Bias */
 	display(mid7win, 2, 16, "%lu", getbeu32(buf, 16));	/* Estimated Time */
-	/* Not a CSD field, but there's no better place to put it */
-	if (timedelta != 0)
-	    display(mid7win, 2, 39, "%f", timedelta);	/* PPS offset */
 	monitor_log("CSD 0x07=");
 	break;
 
@@ -582,6 +580,15 @@ static void sirf_update(void)
 	(void)wnoutrefresh(mid19win);
     }
     /*@ +nullpass -nullderef @*/
+
+    /* Not a CSD field, but there's no better place to put it */
+    if (pps_thread_lastpps(&session, &drift) > 0) {
+	/*@-type@*/ /* splint is confused about struct timespec */
+	double timedelta = timespec_diff_ns(drift.real, drift.clock) * 1e-9;
+	/*@+type@*/
+	display(mid7win, 2, 39, "%.9f", timedelta);	/* PPS offset */
+	wnoutrefresh(mid7win);
+    }
 }
 
 /*@ +globstate */
diff --git a/monitor_ubx.c b/monitor_ubx.c
index e4e550d2..827a7c0e 100644
--- a/monitor_ubx.c
+++ b/monitor_ubx.c
@@ -229,6 +229,7 @@ static void ubx_update(void)
     unsigned char *buf;
     size_t data_len;
     unsigned short msgid;
+    struct timedrift_t drift;
 
     buf = session.packet.outbuffer;
     msgid = (unsigned short)((buf[2] << 8) | buf[3]);
@@ -247,8 +248,13 @@ static void ubx_update(void)
 	break;
     }
 
-    if (timedelta != 0)
-	(void)mvwprintw(ppswin, 1, 13, "%f", timedelta);
+    if (pps_thread_lastpps(&session, &drift) > 0) {
+	/*@-type@*/ /* splint is confused about struct timespec */
+	double timedelta = timespec_diff_ns(drift.real, drift.clock) * 1e-9;
+	/*@+type@*/
+	(void)mvwprintw(ppswin, 1, 13, "%.9f", timedelta);
+	wnoutrefresh(ppswin);
+    }
 }
 
 static int ubx_command(char line[]UNUSED)
diff --git a/ppsthread.c b/ppsthread.c
index 8f7c19c9..31ac30db 100644
--- a/ppsthread.c
+++ b/ppsthread.c
@@ -56,6 +56,8 @@
 #include <glob.h>
 #endif
 
+static pthread_mutex_t ppslast_mutex;
+
 #if defined(HAVE_SYS_TIMEPPS_H)
 /*@-compdestroy -nullpass -unrecog@*/
 static int init_kernel_pps(struct gps_device_t *session)
@@ -494,6 +496,14 @@ static /*@null@*/ void *gpsd_ppsmonitor(void *arg)
 		if (session->context->pps_hook != NULL)
 		    session->context->pps_hook(session, &drift);
 		/*@+compdef@*/
+		/*@ -unrecog  (splint has no pthread declarations as yet) @*/
+		(void)pthread_mutex_lock(&ppslast_mutex);
+		/*@ +unrecog @*/
+		session->ppslast = drift;
+		session->ppscount++;
+		/*@ -unrecog (splint has no pthread declarations as yet) @*/
+		(void)pthread_mutex_unlock(&ppslast_mutex);
+		/*@ +unrecog @*/
             }
 	    gpsd_report(session->context->debug, LOG_INF,
 		    "PPS edge %.20s %lu.%09lu offset %.9f\n",
@@ -552,6 +562,24 @@ void pps_thread_deactivate(struct gps_device_t *session)
     /*@+nullstate +mustfreeonly@*/
 }
 
+int pps_thread_lastpps(struct gps_device_t *session, struct timedrift_t *td)
+/* return a copy of the drift at the time of the last PPS */
+{
+    volatile int ret;
+
+    /*@ -unrecog  (splint has no pthread declarations as yet) @*/
+    (void)pthread_mutex_lock(&ppslast_mutex);
+    /*@ +unrecog @*/
+    *td = session->ppslast;
+    ret = session->ppscount;
+    gpsd_release_reporting_lock();
+    /*@ -unrecog (splint has no pthread declarations as yet) @*/
+    (void)pthread_mutex_unlock(&ppslast_mutex);
+    /*@ +unrecog @*/
+
+    return ret;
+}
+
 #endif /* PPS_ENABLE */
 
 /* end */