[ntp:bk-ntp-dev-send] BitKeeper diffs

Harlan Stenn stenn at deacon.udel.edu
Mon Apr 4 00:30:35 UTC 2011


#### ChangeSet ####
2011-04-03 19:25:33+00:00, davehart at shiny.ad.hartbrothers.com
  Cleanup a few Coverity static analysis warnings.

==== include/ntp_config.h ====
2011-04-03 19:25:30+00:00, davehart at shiny.ad.hartbrothers.com +3 -2
  Add prototype for check_gen_fifo_consistency() overlooked in prior cet.

--- 1.70/include/ntp_config.h	2010-11-28 14:24:28 -05:00
+++ 1.71/include/ntp_config.h	2011-04-03 15:25:30 -04:00
@@ -289,8 +289,9 @@ typedef struct settrap_parms_tag {
 const char * token_name(int token);
 
 /* generic fifo routines for structs linked by 1st member */
-void *append_gen_fifo(void *fifo, void *entry);
-void *concat_gen_fifos(void *first, void *second);
+void	check_gen_fifo_consistency(void *fifo);
+void*	append_gen_fifo(void *fifo, void *entry);
+void *	concat_gen_fifos(void *first, void *second);
 #define APPEND_G_FIFO(pf, pe)		\
 	((pf) = append_gen_fifo((pf), (pe)))
 #define CONCAT_G_FIFOS(first, second)	\

==== lib/isc/task.c ====
2011-04-03 19:25:30+00:00, davehart at shiny.ad.hartbrothers.com +1 -1
  shuffle lock destruction until after protected field update to quiet
    Coverity's lock analysis.

--- 1.3/lib/isc/task.c	2011-03-16 21:25:11 -04:00
+++ 1.4/lib/isc/task.c	2011-04-03 15:25:30 -04:00
@@ -1024,8 +1024,8 @@ manager_free(isc_taskmgr_t *manager) {
 	(void)isc_condition_destroy(&manager->work_available);
 	isc_mem_free(manager->mctx, manager->threads);
 #endif /* ISC_PLATFORM_USETHREADS */
-	DESTROYLOCK(&manager->lock);
 	manager->magic = 0;
+	DESTROYLOCK(&manager->lock);
 	mctx = manager->mctx;
 	isc_mem_put(mctx, manager, sizeof(*manager));
 	isc_mem_detach(&mctx);

==== libntp/audio.c ====
2011-04-03 19:25:30+00:00, davehart at shiny.ad.hartbrothers.com +4 -4
  Use width with sscanf(str, "%s", ...) to avoid overrun of destination.

--- 1.28/libntp/audio.c	2010-02-16 04:40:11 -05:00
+++ 1.29/libntp/audio.c	2011-04-03 15:25:30 -04:00
@@ -175,19 +175,19 @@ audio_config_read(
 			continue;
 
 		if (!strncmp(cc, "IDEV", (size_t) 4)) {
-			sscanf(ca, "%s", ab);
+			sscanf(ca, "%99s", ab);
 			strncpy(cf_i_dev, ab, sizeof(cf_i_dev));
 			printf("idev <%s>\n", ab);
 		} else if (!strncmp(cc, "CDEV", (size_t) 4)) {
-			sscanf(ca, "%s", ab);
+			sscanf(ca, "%99s", ab);
 			strncpy(cf_c_dev, ab, sizeof(cf_c_dev));
 			printf("cdev <%s>\n", ab);
 		} else if (!strncmp(cc, "AGC", (size_t) 3)) {
-			sscanf(ca, "%s", ab);
+			sscanf(ca, "%99s", ab);
 			strncpy(cf_agc, ab, sizeof(cf_agc));
 			printf("agc <%s> %d\n", ab, i);
 		} else if (!strncmp(cc, "MONITOR", (size_t) 7)) {
-			sscanf(ca, "%s", ab);
+			sscanf(ca, "%99s", ab);
 			strncpy(cf_monitor, ab, sizeof(cf_monitor));
 			printf("monitor <%s> %d\n", ab, mixer_name(ab, -1));
 		}

==== libntp/work_thread.c ====
2011-04-03 19:25:30+00:00, davehart at shiny.ad.hartbrothers.com +3 -3
  Change several DEBUG_REQUIRE to REQUIRE so needed safety checks are
    included in -nodebug builds.

--- 1.6/libntp/work_thread.c	2011-03-05 17:35:33 -05:00
+++ 1.7/libntp/work_thread.c	2011-04-03 15:25:30 -04:00
@@ -204,9 +204,9 @@ send_blocking_req_internal(
 {
 	blocking_pipe_header *	threadcopy;
 
-	DEBUG_REQUIRE(hdr != NULL);
-	DEBUG_REQUIRE(hdr->octets > sizeof(*hdr));
-	DEBUG_REQUIRE(data != NULL);
+	REQUIRE(hdr != NULL);
+	REQUIRE(hdr->octets > sizeof(*hdr));
+	REQUIRE(data != NULL);
 	DEBUG_REQUIRE(BLOCKING_REQ_MAGIC == hdr->magic_sig);
 
 	ensure_workitems_empty_slot(c);

==== ntpd/ntp_config.c ====
2011-04-03 19:25:31+00:00, davehart at shiny.ad.hartbrothers.com +2 -2
  match check_gen_fifo_consistency parameter naame with ntp_config.h

--- 1.286/ntpd/ntp_config.c	2011-04-03 13:35:32 -04:00
+++ 1.287/ntpd/ntp_config.c	2011-04-03 15:25:31 -04:00
@@ -956,13 +956,13 @@ dump_config_tree(
 /* generic fifo routines for structs linked by 1st member */
 #ifdef NTP_DEBUG_LISTS_H
 void
-check_gen_fifo_consistency(void *pfv)
+check_gen_fifo_consistency(void *fifo)
 {
 	gen_fifo *pf;
 	gen_node *pthis;
 	gen_node **pptail;
 
-	pf = pfv;
+	pf = fifo;
 	REQUIRE((NULL == pf->phead && NULL == pf->pptail) ||
 		(NULL != pf->phead && NULL != pf->pptail));
 

==== ntpd/refclock_nmea.c ====
2011-04-03 19:25:31+00:00, davehart at shiny.ad.hartbrothers.com +8 -6
  leave room for terminating NUL by calling readlink with sizeof(...) - 1.

--- 1.59/ntpd/refclock_nmea.c	2011-02-20 15:22:40 -05:00
+++ 1.60/ntpd/refclock_nmea.c	2011-04-03 15:25:31 -04:00
@@ -359,7 +359,7 @@ nmea_start(
 		 *
 		 * ln -s server:port /dev/gps1
 		 */
-		char buffer[80];
+		char buffer[PATH_MAX];
 		char *nmea_host, *nmea_tail;
 		u_long nmea_port;
 		int   len;
@@ -367,14 +367,15 @@ nmea_start(
 		struct addrinfo *ai;
 		int rc;
 	
-		if ((len = readlink(device,buffer,sizeof(buffer))) == -1)
-			return(0);
-		buffer[len] = 0;
+		len = readlink(device, buffer, sizeof(buffer) - 1);
+		if (-1 == len)
+			return 0;
+		buffer[len] = '\0';
 
 		if ((nmea_host = strtok(buffer,":")) == NULL)
-			return(0);
+			return 0;
 		if ((nmea_tail = strtok(NULL,":")) == NULL)
-			return(0);
+			return 0;
 		if (!atouint(nmea_tail, &nmea_port) ||
 		    nmea_port > USHRT_MAX)
 			return 0;
@@ -399,6 +400,7 @@ nmea_start(
 			freeaddrinfo(ai);
 			return 0;
 		}
+		/* blocking connect also naughty, see above. */
 		rc = connect(fd, ai->ai_addr, ai->ai_addrlen);
 		freeaddrinfo(ai);
 		if (-1 == rc) {

#### ChangeSet ####
2011-04-03 17:35:33+00:00, davehart at shiny.ad.hartbrothers.com
  [Bug 1840] ntp_lists.h FIFO macros buggy.

==== ChangeLog ====
2011-04-03 17:35:31+00:00, davehart at shiny.ad.hartbrothers.com +1 -0
  [Bug 1840] ntp_lists.h FIFO macros buggy.

--- 1.830/ChangeLog	2011-04-03 02:35:25 -04:00
+++ 1.831/ChangeLog	2011-04-03 13:35:31 -04:00
@@ -1,3 +1,4 @@
+* [Bug 1840] ntp_lists.h FIFO macros buggy.
 (4.2.7p144) 2011/04/03 Released by Harlan Stenn <stenn at ntp.org>
 * [Bug 1874] ntpq -c "rv 0 sys_var_list" empty.
 (4.2.7p143) 2011/03/31 Released by Harlan Stenn <stenn at ntp.org>

==== include/ntp_lists.h ====
2011-04-03 17:35:32+00:00, davehart at shiny.ad.hartbrothers.com +40 -28
  Maintain pptail correctly in UNLINK_FIFO(), CONCAT_FIFO().
  Add more checks to CHECK_FIFO_CONSISTENCY, use it more.
  Re-enable NTP_DEBUG_LISTS_H on DEBUG builds.

--- 1.10/include/ntp_lists.h	2011-02-20 15:47:36 -05:00
+++ 1.11/include/ntp_lists.h	2011-04-03 13:35:32 -04:00
@@ -109,6 +109,10 @@
 #include "ntp_types.h"		/* TRUE and FALSE */
 #include "ntp_assert.h"
 
+#ifdef DEBUG
+# define NTP_DEBUG_LISTS_H
+#endif
+
 /*
  * If list debugging is not enabled, save a little time by not clearing
  * an entry's link pointer when it is unlinked, as the stale pointer
@@ -116,7 +120,7 @@
  * list.
  */
 #ifndef NTP_DEBUG_LISTS_H
-#define MAYBE_Z_LISTS(p)	do { } while (0)
+#define MAYBE_Z_LISTS(p)	do { } while (FALSE)
 #else
 #define MAYBE_Z_LISTS(p)	(p) = NULL
 #endif
@@ -125,7 +129,7 @@
 do {								\
 	(pentry)->nextlink = (listhead);			\
 	(listhead) = (pentry);					\
-} while (0)
+} while (FALSE)
 
 #define LINK_TAIL_SLIST(listhead, pentry, nextlink, entrytype)	\
 do {								\
@@ -137,7 +141,7 @@ do {								\
 								\
 	(pentry)->nextlink = NULL;				\
 	*pptail = (pentry);					\
-} while (0)
+} while (FALSE)
 
 #define LINK_SORT_SLIST_CURRENT()	(*ppentry)
 #define	L_S_S_CUR()			LINK_SORT_SLIST_CURRENT()
@@ -168,9 +172,9 @@ do {								\
 	(punlinked) = (listhead);				\
 	if (NULL != (punlinked)) {				\
 		(listhead) = (punlinked)->nextlink;		\
-		(punlinked)->nextlink = NULL;			\
+		MAYBE_Z_LISTS((punlinked)->nextlink);		\
 	}							\
-} while (0)
+} while (FALSE)
 
 #define UNLINK_EXPR_SLIST_CURRENT()	(*ppentry)
 #define	U_E_S_CUR()			UNLINK_EXPR_SLIST_CURRENT()
@@ -193,10 +197,10 @@ do {								\
 	if (ppentry != NULL) {					\
 		(punlinked) = *ppentry;				\
 		*ppentry = (punlinked)->nextlink;		\
-		(punlinked)->nextlink = NULL;			\
+		MAYBE_Z_LISTS((punlinked)->nextlink);		\
 	} else							\
 		(punlinked) = NULL;				\
-} while (0)
+} while (FALSE)
 
 #define UNLINK_SLIST(punlinked, listhead, ptounlink, nextlink,	\
 		     entrytype)					\
@@ -213,7 +217,7 @@ do {								\
 		NTP_INSIST(pentry != pentry->nextlink);		\
 		NTP_INSIST((listhead) != pentry->nextlink);	\
 	}							\
-} while (0)
+} while (FALSE)
 
 /*
  * FIFO
@@ -234,14 +238,11 @@ struct {							\
  * For DEBUG builds only, verify both or neither of the anchor pointers
  * are NULL with each operation.
  */
-#if !defined(NTP_DEBUG_LISTS_H) || !defined(DEBUG)
-#define	CHECK_FIFO_CONSISTENCY(queue)	do { } while (0);
+#if !defined(NTP_DEBUG_LISTS_H)
+#define	CHECK_FIFO_CONSISTENCY(anchor)	do { } while (FALSE)
 #else
 #define	CHECK_FIFO_CONSISTENCY(anchor)				\
-	NTP_REQUIRE((NULL == (anchor).phead			\
-		     && NULL == (anchor).pptail)		\
-		    || (NULL != (anchor).phead			\
-		     && NULL != (anchor).pptail))
+	check_gen_fifo_consistency(&(anchor))
 #endif
 
 #define LINK_FIFO(anchor, pentry, nextlink)			\
@@ -252,12 +253,14 @@ do {								\
 	if (NULL != (anchor).pptail) {				\
 		(*((anchor).pptail))->nextlink = (pentry);	\
 		(anchor).pptail =				\
-		    &((*((anchor).pptail))->nextlink);		\
+		    &(*((anchor).pptail))->nextlink;		\
 	} else {						\
 		(anchor).phead = (pentry);			\
-		(anchor).pptail = &((anchor).phead);		\
+		(anchor).pptail = &(anchor).phead;		\
 	}							\
-} while (0)
+								\
+	CHECK_FIFO_CONSISTENCY(anchor);				\
+} while (FALSE)
 
 #define UNLINK_FIFO(punlinked, anchor, nextlink)		\
 do {								\
@@ -268,9 +271,13 @@ do {								\
 		(anchor).phead = (punlinked)->nextlink;		\
 		if (NULL == (anchor).phead)			\
 			(anchor).pptail = NULL;			\
+		else if ((anchor).pptail ==			\
+			 &(punlinked)->nextlink)		\
+			(anchor).pptail = &(anchor).phead;	\
 		MAYBE_Z_LISTS((punlinked)->nextlink);		\
+		CHECK_FIFO_CONSISTENCY(anchor);			\
 	}							\
-} while (0)
+} while (FALSE)
 
 #define CONCAT_FIFO(f1, f2, nextlink)				\
 do {								\
@@ -280,14 +287,19 @@ do {								\
 	if ((f2).pptail != NULL) {				\
 		if ((f1).pptail != NULL) {			\
 			(*(f1).pptail)->nextlink = (f2).phead;	\
-			(f1).pptail = (f2).pptail;		\
+			if ((f2).pptail == &(f2).phead)		\
+				(f1).pptail =			\
+				    &(*(f1).pptail)->nextlink;	\
+			else					\
+				(f1).pptail = (f2).pptail;	\
+			CHECK_FIFO_CONSISTENCY(f1);		\
 		} else	{					\
 			(f1) = (f2);				\
 		}						\
-		(f2).phead = NULL;				\
-		(f2).pptail = NULL;				\
+		MAYBE_Z_LISTS((f2).phead);			\
+		MAYBE_Z_LISTS((f2).pptail);			\
 	}							\
-} while (0)
+} while (FALSE)
 
 /*
  * DLIST
@@ -302,7 +314,7 @@ struct {							\
 do {								\
 	(listhead).link.f = &(listhead);			\
 	(listhead).link.b = &(listhead);			\
-} while (0)
+} while (FALSE)
 
 #define HEAD_DLIST(listhead, link)				\
 	(							\
@@ -338,7 +350,7 @@ do {								\
 	(pentry)->link.b = &(listhead);				\
 	(listhead).link.f->link.b = (pentry);			\
 	(listhead).link.f = (pentry);				\
-} while (0)
+} while (FALSE)
 
 #define LINK_TAIL_DLIST(listhead, pentry, link)			\
 do {								\
@@ -346,15 +358,15 @@ do {								\
 	(pentry)->link.f = &(listhead);				\
 	(listhead).link.b->link.f = (pentry);			\
 	(listhead).link.b = (pentry);				\
-} while (0)
+} while (FALSE)
 
 #define UNLINK_DLIST(ptounlink, link)				\
 do {								\
 	(ptounlink)->link.b->link.f = (ptounlink)->link.f;	\
 	(ptounlink)->link.f->link.b = (ptounlink)->link.b;	\
-	(ptounlink)->link.b = NULL;				\
-	(ptounlink)->link.f = NULL;				\
-} while (0)
+	MAYBE_Z_LISTS((ptounlink)->link.b);			\
+	MAYBE_Z_LISTS((ptounlink)->link.f);			\
+} while (FALSE)
 
 #define ITER_DLIST_BEGIN(listhead, iter, link, entrytype)	\
 {								\

==== libntp/mktime.c ====
2011-04-03 17:35:32+00:00, davehart at shiny.ad.hartbrothers.com +2 -2
  whitespace only (leading blanks -> tab)

--- 1.9/libntp/mktime.c	2010-01-27 13:45:24 -05:00
+++ 1.10/libntp/mktime.c	2011-04-03 13:35:32 -04:00
@@ -227,9 +227,9 @@ time2(
 	t = (t < 0) ? 0 : ((time_t) 1 << bits);
 	for ( ; ; ) {
 		if (usezn)
-	        	mytm = *localtime(&t);
+			mytm = *localtime(&t);
 		else
-	        	mytm = *gmtime(&t);
+			mytm = *gmtime(&t);
 		dir = tmcomp(&mytm, &yourtm);
 		if (dir != 0) {
 			if (bits-- < 0)

==== ntpd/ntp_config.c ====
2011-04-03 17:35:32+00:00, davehart at shiny.ad.hartbrothers.com +31 -0
  Move CHECK_FIFO_CONSISTENCY guts from macro to function so debuggers
    can highlight the failing check rather than entire macro invocation.

--- 1.285/ntpd/ntp_config.c	2011-03-15 22:09:54 -04:00
+++ 1.286/ntpd/ntp_config.c	2011-04-03 13:35:32 -04:00
@@ -954,6 +954,30 @@ dump_config_tree(
 
 
 /* generic fifo routines for structs linked by 1st member */
+#ifdef NTP_DEBUG_LISTS_H
+void
+check_gen_fifo_consistency(void *pfv)
+{
+	gen_fifo *pf;
+	gen_node *pthis;
+	gen_node **pptail;
+
+	pf = pfv;
+	REQUIRE((NULL == pf->phead && NULL == pf->pptail) ||
+		(NULL != pf->phead && NULL != pf->pptail));
+
+	pptail = &pf->phead;
+	for (pthis = pf->phead;
+	     pthis != NULL;
+	     pthis = pthis->link)
+		if (NULL != pthis->link)
+			pptail = &pthis->link;
+
+	REQUIRE(NULL == pf->pptail || pptail == pf->pptail);
+}
+#endif	/* NTP_DEBUG_LISTS_H */
+
+
 void *
 append_gen_fifo(
 	void *fifo,
@@ -967,8 +991,11 @@ append_gen_fifo(
 	pe = entry;
 	if (NULL == pf)
 		pf = emalloc_zero(sizeof(*pf));
+	else
+		CHECK_FIFO_CONSISTENCY(*pf);
 	if (pe != NULL)
 		LINK_FIFO(*pf, pe, link);
+	CHECK_FIFO_CONSISTENCY(*pf);
 
 	return pf;
 }
@@ -990,6 +1017,8 @@ concat_gen_fifos(
 	else if (NULL == pf2)
 		return pf1;
 
+	CHECK_FIFO_CONSISTENCY(*pf1);
+	CHECK_FIFO_CONSISTENCY(*pf2);
 	CONCAT_FIFO(*pf1, *pf2, link);
 	free(pf2);
 
@@ -1162,6 +1191,8 @@ create_peer_node(
 	 * FIFO.  The options FIFO is consumed and reclaimed here.
 	 */
 
+	if (options != NULL)
+		CHECK_FIFO_CONSISTENCY(*options);
 	while (options != NULL) {
 		UNLINK_FIFO(option, *options, link);
 		if (NULL == option) {


More information about the bk-ntp-dev-send mailing list