From bab8757bc9e0b64883c8eeaa9ce5bd1909bbdcd4 Mon Sep 17 00:00:00 2001 From: "Eric S. Raymond" Date: Thu, 26 Feb 2015 14:28:33 -0500 Subject: Pre-release splint/cppcheck/coverity cleanup. All regression tests pass. --- NEWS | 1 + SConstruct | 3 ++- drivers.c | 6 +++++- gpsmon.c | 2 +- ntpmon.c | 20 ++++++++++++++------ ntpshm.c | 1 + ntpshm.h | 4 ++-- ntpshmread.c | 18 +++++++++++++----- ppsthread.c | 4 ++++ test_mktime.c | 1 + 10 files changed, 44 insertions(+), 16 deletions(-) diff --git a/NEWS b/NEWS index b460eab4..6a1de183 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,7 @@ GPSD project news Repository head: + compiler.h inclusion removed for gps.h so it's standalone for /usr/include. TOFF JSON report gives the offset between GPS top of second and clock time. A new ntpmon tool supports capturing clock samples from NTP SHM segments. diff --git a/SConstruct b/SConstruct index 15289fd0..9006f5c6 100644 --- a/SConstruct +++ b/SConstruct @@ -1485,6 +1485,7 @@ splint_table = [ ('splint-gps2udp',['gps2udp.c'],'gps2udp', ['']), ('splint-gpsdecode',['gpsdecode.c'],'gpsdecode', ['']), ('splint-gpxlogger',['gpxlogger.c'],'gpxlogger', ['']), + ('splint-ntpmon',['ntpmon.c'],'ntpmon', ['']), ('splint-test_packet',['test_packet.c'],'test_packet test harness', ['']), ('splint-test_mktime',['test_mktime.c'],'test_mktime test harness', ['']), ('splint-test_geoid',['test_geoid.c'],'test_geoid test harness', ['']), @@ -1927,7 +1928,7 @@ distclean = env.Alias('distclean', [clean, testclean, webclean]) # Tags for Emacs and vi misc_sources = ['cgps.c', 'gpsctl.c', 'gpsdctl.c', 'gpspipe.c', - 'gps2udp.c', 'gpsdecode.c', 'gpxlogger.c'] + 'gps2udp.c', 'gpsdecode.c', 'gpxlogger.c', 'ntpmon'] sources = libgpsd_sources + libgps_sources \ + gpsd_sources + gpsmon_sources + misc_sources env.Command('TAGS', sources, ['etags ' + " ".join(sources)]) diff --git a/drivers.c b/drivers.c index 6b993c4f..576ef574 100644 --- a/drivers.c +++ b/drivers.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #ifndef S_SPLINT_S #include @@ -1417,7 +1418,6 @@ const struct gps_type_t driver_aivdm = { static void path_rewrite(struct gps_device_t *session, char *prefix) /* prepend the session path to the value of a specified attribute */ { - /* * Hack the packet to reflect its origin. This code is supposed * to insert the path naming the remote gpsd instance into the @@ -1426,6 +1426,10 @@ static void path_rewrite(struct gps_device_t *session, char *prefix) */ char *prefloc; +#ifdef S_SPLINT_S + assert(prefix != NULL && session->lexer.outbuffer != NULL); +#endif /* S_SPLINT_S */ + /* possibly the rewrite has been done already, this comw up in gpsmon */ if (strstr((char *)session->lexer.outbuffer, session->gpsdata.dev.path) != NULL) return; diff --git a/gpsmon.c b/gpsmon.c index cb0c0f67..106f3a1c 100644 --- a/gpsmon.c +++ b/gpsmon.c @@ -329,7 +329,7 @@ static void gpsmon_report(const char *buf) #ifdef PPS_ENABLE /*@-compdef@*/ -static void packet_vlog(/*@out@*/char *buf, size_t len, const char *fmt, va_list ap) +static void packet_vlog(char *buf, size_t len, const char *fmt, va_list ap) { char buf2[BUFSIZ]; diff --git a/ntpmon.c b/ntpmon.c index c822f554..7899fd77 100644 --- a/ntpmon.c +++ b/ntpmon.c @@ -23,8 +23,9 @@ static void shm_shutdown(void) { struct shmTime **pp; - for (pp = segments; *pp; pp++) - (void)shmdt((void *)(*pp)); + for (pp = segments; pp < segments + NTPSEGMENTS; pp++) + if (*pp != NULL) + (void)shmdt((void *)(*pp)); } int main(int argc, char **argv) @@ -45,7 +46,7 @@ int main(int argc, char **argv) fprintf(stderr, "ntpmon: zero units declared.\n"); exit(EXIT_FAILURE); } - break; + //0break; case 'v': verbose = true; break; @@ -66,7 +67,6 @@ int main(int argc, char **argv) for (;;) { struct shm_stat_t shm_stat; - int i; for (i = 0; i < NTPSEGMENTS; i++) { enum segstat_t status = shm_query(segments[i], &shm_stat); @@ -75,6 +75,8 @@ int main(int argc, char **argv) switch(status) { case OK: + /*@-mustfreefresh -formattype@*/ + /*@-type@*//* splint is confused about struct timespec */ if (shm_stat.tvc.tv_sec != tick[i].tv_sec || shm_stat.tvc.tv_nsec != tick[i].tv_nsec) { printf("sample %s %ld.%09ld %ld.%09ld %ld.%09ld %d %3d\n", shm_name(i), @@ -84,6 +86,8 @@ int main(int argc, char **argv) shm_stat.leap, shm_stat.precision); tick[i] = shm_stat.tvc; } + /*@+type@*/ + /*@+mustfreefresh +formattype@*/ break; case NO_SEGMENT: break; @@ -91,15 +95,19 @@ int main(int argc, char **argv) /* do nothing, data not ready, wait another cycle */ break; case BAD_MODE: + /*@-mustfreefresh@*/ fprintf(stderr, "ntpmon: unknown mode %d on segment %s\n", shm_stat.status, shm_name(i)); + /*@+mustfreefresh@*/ break; case CLASH: /* do nothing, data is corrupt, wait another cycle */ break; default: + /*@-mustfreefresh@*/ fprintf(stderr, "ntpmon: unknown status %d on segment %s\n", status, shm_name(i)); + /*@+mustfreefresh@*/ break; } } @@ -110,10 +118,10 @@ int main(int argc, char **argv) * we're ignoring duplicates via timestamp, polling * at interval < 1 sec shouldn't be a problem. */ - usleep(1000); + (void)usleep(1000); } - exit(EXIT_SUCCESS); + //exit(EXIT_SUCCESS); } /* end */ diff --git a/ntpshm.c b/ntpshm.c index 1abfdf3b..0d11a089 100644 --- a/ntpshm.c +++ b/ntpshm.c @@ -361,6 +361,7 @@ static void chrony_send(struct gps_device_t *session, struct timedelta_t *td) sample.pulse = 0; sample.leap = session->context->leap_notify; sample.magic = SOCK_MAGIC; + /*@-compdef@*/ /*@-type@*//* splint is confused about struct timespec */ /* chronyd wants a timeval, not a timspec, not to worry, it is * just the top of the second */ diff --git a/ntpshm.h b/ntpshm.h index ceb01526..c8cd3b6e 100644 --- a/ntpshm.h +++ b/ntpshm.h @@ -66,8 +66,8 @@ struct shm_stat_t { int leap; }; -struct shmTime *shm_get(int, bool, bool); +struct shmTime /*@null@*/ *shm_get(int, bool, bool); extern char *shm_name(const int); -enum segstat_t shm_query(struct shmTime *, struct shm_stat_t *); +enum segstat_t shm_query(/*@null@*/struct shmTime *, /*@out@*/struct shm_stat_t *); /* end */ diff --git a/ntpshmread.c b/ntpshmread.c index 5d39dad6..880ef542 100644 --- a/ntpshmread.c +++ b/ntpshmread.c @@ -12,16 +12,19 @@ #include #include #include -#include #include #include +#ifndef S_SPLINT_S +#include +#endif /* S_SPLINT_S*/ #include "ntpshm.h" #include "compiler.h" -struct shmTime *shm_get(const int unit, const bool create, const bool forall) +struct shmTime /*@null@*/ *shm_get(const int unit, const bool create, const bool forall) /* initialize an initial segment */ { + /*@-mustfreefresh@*/ struct shmTime *p = NULL; int shmid; @@ -29,7 +32,7 @@ struct shmTime *shm_get(const int unit, const bool create, const bool forall) * Big units will give non-ascii but that's OK * as long as everybody does it the same way. */ - shmid = shmget(NTPD_BASE + unit, sizeof(struct shmTime), + shmid = shmget((key_t)(NTPD_BASE + unit), sizeof(struct shmTime), (create ? IPC_CREAT : 0) | (forall ? 0666 : 0600)); if (shmid == -1) { /* error */ return NULL; @@ -39,19 +42,22 @@ struct shmTime *shm_get(const int unit, const bool create, const bool forall) return NULL; } return p; + /*@+mustfreefresh@*/ } +/*@-statictrans@*/ char *shm_name(const int unit) /* return the name of a specified segment */ { static char name[5] = "NTP\0"; - name[3] = '0' + (char)unit; + /*@i2@*/name[3] = '0' + (char)unit; return name; } +/*@+statictrans@*/ -enum segstat_t shm_query(struct shmTime *shm_in, struct shm_stat_t *shm_stat) +enum segstat_t shm_query(/*@null@*/struct shmTime *shm_in, /*@out@*/struct shm_stat_t *shm_stat) /* try to grab a sample from the specified SHM segment */ { volatile struct shmTime shmcopy, *shm = shm_in; @@ -64,6 +70,7 @@ enum segstat_t shm_query(struct shmTime *shm_in, struct shm_stat_t *shm_stat) return NO_SEGMENT; } + /*@-type@*//* splint is confused about struct timespec */ shm_stat->tvc.tv_sec = shm_stat->tvc.tv_nsec = 0; clock_gettime(CLOCK_REALTIME, &shm_stat->tvc); @@ -155,6 +162,7 @@ enum segstat_t shm_query(struct shmTime *shm_in, struct shm_stat_t *shm_stat) shm_stat->status = BAD_MODE; break; } + /*@-type@*/ /* * leap field is not a leap offset but a leap notification code. diff --git a/ppsthread.c b/ppsthread.c index 7bd5d1d1..ad8750a4 100644 --- a/ppsthread.c +++ b/ppsthread.c @@ -673,9 +673,11 @@ static /*@null@*/ void *gpsd_ppsmonitor(void *arg) /* check to see if we have a fresh timestamp from the * GPS serial input then use that */ + /*@-compdef@*/ TS_SUB( &offset, &ppstimes.real, &ppstimes.clock); TS_SUB( &delay, &ppstimes.clock, &last_fixtime_clock); timespec_str( &delay, delay_str, sizeof(delay_str) ); + /*@+compdef@*/ if ( 0> delay.tv_sec || 0 > delay.tv_nsec ) { gpsd_report(&session->context->errout, LOG_RAW, @@ -727,6 +729,7 @@ static /*@null@*/ void *gpsd_ppsmonitor(void *arg) /*@+type@*/ /*@+compdef@*/ } + /*@-compdef@*/ /*@-type@*/ /* splint is confused about struct timespec */ timespec_str( &clock_ts, ts_str1, sizeof(ts_str1) ); timespec_str( &offset, offset_str, sizeof(offset_str) ); @@ -734,6 +737,7 @@ static /*@null@*/ void *gpsd_ppsmonitor(void *arg) "PPS edge %.20s @ %s offset %.20s\n", log1, ts_str1, offset_str); /*@+type@*/ + /*@+compdef@*/ } else { gpsd_report(&session->context->errout, LOG_RAW, "PPS edge rejected %.100s", log); diff --git a/test_mktime.c b/test_mktime.c index 32af8837..0c3244c6 100644 --- a/test_mktime.c +++ b/test_mktime.c @@ -8,6 +8,7 @@ #include #include "gps.h" +#include "compiler.h" /*@-type@*/ static struct -- cgit v1.2.1