[Git][NTPsec/ntpsec][master] Attempt to fix signed-vs.-unsigned issues revealed by -Wextra.

Eric S. Raymond gitlab at mg.gitlab.com
Sun Nov 22 21:33:08 UTC 2015


Eric S. Raymond pushed to branch master at NTPsec / ntpsec


Commits:
5aa9538e by Eric S. Raymond at 2015-11-22T16:18:13Z
Attempt to fix signed-vs.-unsigned issues revealed by -Wextra.

The theme in this patch is changing variables that are semantically data
lengths from int to size_t.  This cleanly resolves many sign-compare bugs.
It requires some printfs to change from %d to %zd formats.

No logic changes. Associated code has been eyeball-checked for subtractions that
might induce modular wraparound, but if a future core dump produces a bisection
that lands here that will be the thing to suspect.

- - - - -


12 changed files:

- include/ntp_refclock.h
- include/recvbuff.h
- ntpd/ntp_control.c
- ntpd/ntp_proto.c
- ntpd/ntp_refclock.c
- ntpd/refclock_acts.c
- ntpd/refclock_arc.c
- ntpd/refclock_irig.c
- ntpd/refclock_jjy.c
- ntpd/refclock_parse.c
- ntpd/refclock_tsyncpci.c
- ntpq/ntpq.c


Changes:

=====================================
include/ntp_refclock.h
=====================================
--- a/include/ntp_refclock.h
+++ b/include/ntp_refclock.h
@@ -235,7 +235,7 @@ extern 	void	refclock_process_offset(struct refclockproc *, l_fp,
 					l_fp, double);
 extern	void	refclock_report	(struct peer *, int);
 extern	int	refclock_gtlin	(struct recvbuf *, char *, int, l_fp *);
-extern	int	refclock_gtraw	(struct recvbuf *, char *, int, l_fp *);
+extern	size_t	refclock_gtraw	(struct recvbuf *, char *, size_t, l_fp *);
 extern	bool	indicate_refclock_packet(struct refclockio *,
 					 struct recvbuf *);
 extern	void	process_refclock_packet(struct recvbuf *);


=====================================
include/recvbuff.h
=====================================
--- a/include/recvbuff.h
+++ b/include/recvbuff.h
@@ -66,7 +66,7 @@ struct recvbuf {
 	int		msg_flags;	/* Flags received about the packet */
 	l_fp		recv_time;	/* time of arrival */
 	void		(*receiver)(struct recvbuf *); /* callback */
-	u_int		recv_length;	/* number of octets received */
+	size_t		recv_length;	/* number of octets received */
 	union {
 		struct pkt	X_recv_pkt;
 		uint8_t		X_recv_buffer[RX_BUFF_SIZE];


=====================================
ntpd/ntp_control.c
=====================================
--- a/ntpd/ntp_control.c
+++ b/ntpd/ntp_control.c
@@ -1002,7 +1002,7 @@ process_control(
 	dataend = &rpkt.u.data[CTL_MAX_DATA_LEN];
 
 	if ((rbufp->recv_length & 0x3) != 0)
-		DPRINTF(3, ("Control packet length %d unrounded\n",
+		DPRINTF(3, ("Control packet length %zd unrounded\n",
 			    rbufp->recv_length));
 
 	/*
@@ -1027,7 +1027,7 @@ process_control(
 		res_authenticate = true;
 		pkid = (void *)((char *)pkt + properlen);
 		res_keyid = ntohl(*pkid);
-		DPRINTF(3, ("recv_len %d, properlen %d, wants auth with keyid %08x, MAC length=%zu\n",
+		DPRINTF(3, ("recv_len %zd, properlen %d, wants auth with keyid %08x, MAC length=%zu\n",
 			    rbufp->recv_length, properlen, res_keyid,
 			    maclen));
 


=====================================
ntpd/ntp_proto.c
=====================================
--- a/ntpd/ntp_proto.c
+++ b/ntpd/ntp_proto.c
@@ -391,7 +391,7 @@ receive(
 	uint8_t	hisstratum;		/* packet stratum */
 	u_short	restrict_mask;		/* restrict bits */
 	int	has_mac;		/* length of MAC field */
-	int	authlen;		/* offset of MAC field */
+	size_t	authlen;		/* offset of MAC field */
 	int	is_authentic = 0;	/* cryptosum ok */
 	int	retcode = AM_NOMATCH;	/* match code */
 	keyid_t	skeyid = 0;		/* key IDs */
@@ -671,7 +671,7 @@ receive(
 #ifdef DEBUG
 		if (debug)
 			printf(
-			    "receive: at %ld %s<-%s mode %d len %d\n",
+			    "receive: at %ld %s<-%s mode %d len %zd\n",
 			    current_time, stoa(dstadr_sin),
 			    stoa(&rbufp->recv_srcadr), hismode,
 			    authlen);
@@ -682,7 +682,7 @@ receive(
 #ifdef DEBUG
 		if (debug)
 			printf(
-			    "receive: at %ld %s<-%s mode %d keyid %08x len %d auth %d\n",
+			    "receive: at %ld %s<-%s mode %d keyid %08x len %zd auth %d\n",
 			    current_time, stoa(dstadr_sin),
 			    stoa(&rbufp->recv_srcadr), hismode, skeyid,
 			    authlen + has_mac, is_authentic);
@@ -778,7 +778,7 @@ receive(
 			 * purposes is zero. Note the hash is saved for
 			 * use later in the autokey mambo.
 			 */
-			if (authlen > (int)LEN_PKT_NOMAC && pkeyid != 0) {
+			if (authlen > LEN_PKT_NOMAC && pkeyid != 0) {
 				session_key(&rbufp->recv_srcadr,
 				    dstadr_sin, skeyid, 0, 2);
 				tkeyid = session_key(
@@ -812,7 +812,7 @@ receive(
 #ifdef DEBUG
 		if (debug)
 			printf(
-			    "receive: at %ld %s<-%s mode %d keyid %08x len %d auth %d\n",
+			    "receive: at %ld %s<-%s mode %d keyid %08x len %zd auth %d\n",
 			    current_time, stoa(dstadr_sin),
 			    stoa(&rbufp->recv_srcadr), hismode, skeyid,
 			    authlen + has_mac, is_authentic);
@@ -1147,7 +1147,7 @@ receive(
 			if (debug) {
 				 printf(
 					 "receive: at %ld refusing to mobilize passive association"
-					 " with unknown peer %s mode %d keyid %08x len %d auth %d\n",
+					 " with unknown peer %s mode %d keyid %08x len %zd auth %d\n",
 					 current_time, stoa(&rbufp->recv_srcadr), hismode, skeyid,
 					 authlen + has_mac, is_authentic);
 			}
@@ -3491,7 +3491,7 @@ fast_xmit(
 	struct pkt xpkt;	/* transmit packet structure */
 	struct pkt *rpkt;	/* receive packet structure */
 	l_fp	xmt_tx, xmt_ty;
-	int	sendlen;
+	size_t	sendlen;
 #ifdef ENABLE_AUTOKEY
 	uint32_t	temp32;
 #endif


=====================================
ntpd/ntp_refclock.c
=====================================
--- a/ntpd/ntp_refclock.c
+++ b/ntpd/ntp_refclock.c
@@ -626,11 +626,11 @@ refclock_gtlin(
  *
  * *tsptr receives a copy of the buffer timestamp.
  */
-int
+size_t
 refclock_gtraw(
 	struct recvbuf *rbufp,	/* receive buffer pointer */
 	char	*lineptr,	/* current line pointer */
-	int	bmax,		/* remaining characters in line */
+	size_t	bmax,		/* remaining characters in line */
 	l_fp	*tsptr		/* pointer to timestamp returned */
 	)
 {
@@ -643,7 +643,7 @@ refclock_gtraw(
 	lineptr[bmax] = '\0';
 
 	*tsptr = rbufp->recv_time;
-	DPRINTF(2, ("refclock_gtraw: fd %d time %s timecode %d %s\n",
+	DPRINTF(2, ("refclock_gtraw: fd %d time %s timecode %zd %s\n",
 		    rbufp->fd, ulfptoa(&rbufp->recv_time, 6), bmax,
 		    lineptr));
 	return (bmax);


=====================================
ntpd/refclock_acts.c
=====================================
--- a/ntpd/refclock_acts.c
+++ b/ntpd/refclock_acts.c
@@ -317,7 +317,7 @@ acts_receive(
 	struct peer *peer;
 	char	tbuf[sizeof(up->buf)];
 	char *	tptr;
-	u_int	octets;
+	size_t	octets;
 
 	/*
 	 * Initialize pointers and read the timecode and timestamp. Note


=====================================
ntpd/refclock_arc.c
=====================================
--- a/ntpd/refclock_arc.c
+++ b/ntpd/refclock_arc.c
@@ -845,7 +845,8 @@ arc_receive(
 	struct refclockproc *pp;
 	struct peer *peer;
 	char c;
-	int i, wday, month, flags, status;
+	int wday, month, flags, status;
+	size_t i;
 	int arc_last_offset;
 	static int quality_average = 0;
 	static int quality_sum = 0;
@@ -913,7 +914,7 @@ arc_receive(
 		timestamp = rbufp->recv_time;
 #ifdef DEBUG
 		if(debug) { /* Show \r as `R', other non-printing char as `?'. */
-			printf("arc: stamp -->%c<-- (%d chars rcvd)\n",
+			printf("arc: stamp -->%c<-- (%zd chars rcvd)\n",
 			       ((c == '\r') ? 'R' : (isgraph((unsigned char)c) ? c : '?')),
 			       rbufp->recv_length);
 		}
@@ -944,7 +945,7 @@ arc_receive(
 #ifdef DEBUG
 		if(debug > 1) {
 			printf(
-				"arc: %s%d char(s) rcvd, the last for lastcode[%d]; -%sms offset applied.\n",
+				"arc: %s%zd char(s) rcvd, the last for lastcode[%d]; -%sms offset applied.\n",
 				((rbufp->recv_length > 1) ? "*** " : ""),
 				rbufp->recv_length,
 				arc_last_offset,


=====================================
ntpd/refclock_irig.c
=====================================
--- a/ntpd/refclock_irig.c
+++ b/ntpd/refclock_irig.c
@@ -416,7 +416,7 @@ irig_receive(
 	 */
 	double	sample;		/* codec sample */
 	uint8_t	*dpt;		/* buffer pointer */
-	int	bufcnt;		/* buffer counter */
+	size_t	bufcnt;		/* buffer counter */
 	l_fp	ltemp;		/* l_fp temp */
 
 	peer = rbufp->recv_peer;


=====================================
ntpd/refclock_jjy.c
=====================================
--- a/ntpd/refclock_jjy.c
+++ b/ntpd/refclock_jjy.c
@@ -562,7 +562,7 @@ jjy_receive ( struct recvbuf *rbufp )
 	 */
 	if ( up->linediscipline == LDISC_RAW ) {
 
-		pp->lencode  = refclock_gtraw ( rbufp, pp->a_lastcode, BMAX-1, &tRecvTimestamp ) ;
+		pp->lencode  = (int)refclock_gtraw ( rbufp, pp->a_lastcode, BMAX-1, &tRecvTimestamp ) ;
 		/* 3rd argument can be BMAX, but the coverity scan tool claim "Memory - corruptions  (OVERRUN)" */
 		/* "a_lastcode" is defined as "char a_lastcode[BMAX]" in the ntp_refclock.h */
 		/* To avoid its claim, pass the value BMAX-1. */
@@ -2655,7 +2655,8 @@ jjy_start_telephone ( int unit, struct peer *peer, struct jjyunit *up )
 {
 
 	char	sLog [ 80 ], sFirstThreeDigits [ 4 ] ;
-	int	i, iNumberOfDigitsOfPhoneNumber, iCommaCount, iCommaPosition ;
+	int	iNumberOfDigitsOfPhoneNumber, iCommaCount, iCommaPosition ;
+	size_t	i;
 	int	iFirstThreeDigitsCount ;
 
 	jjy_write_clockstats( peer, JJY_CLOCKSTATS_MARK_JJY, "Refclock: Telephone JJY" ) ;
@@ -2774,7 +2775,7 @@ jjy_receive_telephone ( struct recvbuf *rbufp )
 	struct	refclockproc *pp ;
 	struct	jjyunit      *up ;
 	char	*pBuf ;
-	int	iLen ;
+	size_t	iLen ;
 	short	iPreviousModemState ;
 
 	peer = rbufp->recv_peer ;
@@ -3882,10 +3883,10 @@ modem_receive ( struct recvbuf *rbufp )
 #ifdef DEBUG
 	if ( debug ) {
 		char	sResp [ 40 ] ;
-		int	iCopyLen ;
+		size_t	iCopyLen ;
 		iCopyLen = ( iLen <= sizeof(sResp)-1 ? iLen : sizeof(sResp)-1 ) ;
 		strlcpy( sResp, pBuf, sizeof(sResp) ) ;
-		printf ( "refclock_jjy.c : modem_receive : iLen=%d pBuf=[%s] iModemEvent=%d\n", iCopyLen, sResp, up->iModemEvent ) ;
+		printf ( "refclock_jjy.c : modem_receive : iLen=%zd pBuf=[%s] iModemEvent=%d\n", iCopyLen, sResp, up->iModemEvent ) ;
 	}
 #endif
 	modem_control ( peer, pp, up ) ;


=====================================
ntpd/refclock_parse.c
=====================================
--- a/ntpd/refclock_parse.c
+++ b/ntpd/refclock_parse.c
@@ -1912,8 +1912,8 @@ local_receive(
 	if (rbufp->recv_length != sizeof(parsetime_t))
 	{
 		ERR(ERR_BADIO)
-			msyslog(LOG_ERR,"PARSE receiver #%d: local_receive: bad size (got %d expected %d)",
-				CLK_UNIT(parse->peer), rbufp->recv_length, (int)sizeof(parsetime_t));
+			msyslog(LOG_ERR,"PARSE receiver #%d: local_receive: bad size (got %zd expected %zd)",
+				CLK_UNIT(parse->peer), rbufp->recv_length, sizeof(parsetime_t));
 		parse_event(parse, CEVNT_BADREPLY);
 		return;
 	}


=====================================
ntpd/refclock_tsyncpci.c
=====================================
--- a/ntpd/refclock_tsyncpci.c
+++ b/ntpd/refclock_tsyncpci.c
@@ -554,7 +554,7 @@ static void tsync_poll(int unit, struct peer *peer)
     tmscl = ntohl(tmscl);
 
     // Extract leap second info from ioctl payload and perform byte swapping
-    for (i = 0; i < (sizeof(leapSec) / 4); i++)
+    for (i = 0; i < (int)(sizeof(leapSec) / 4); i++)
     {
         for (j = 0; j < 4; j++)
         {


=====================================
ntpq/ntpq.c
=====================================
--- a/ntpq/ntpq.c
+++ b/ntpq/ntpq.c
@@ -450,7 +450,7 @@ ntpqmain(
 	)
 {
 	u_int ihost;
-	int icmd;
+	size_t icmd;
 	int msglen;
 
 	delay_time.l_ui = 0;
@@ -620,9 +620,10 @@ ntpqmain(
 		getcmds();
 	} else {
 		for (ihost = 0; ihost < numhosts; ihost++) {
+			int i;
 			if (openhost(chosts[ihost].name, chosts[ihost].fam))
-				for (icmd = 0; icmd < numcmds; icmd++)
-					docmd(ccmds[icmd]);
+				for (i = 0; i < numcmds; i++)
+					docmd(ccmds[i]);
 		}
 	}
 #ifdef SYS_WINNT



View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/5aa9538eec5156b624271b53dcc875c1c4673696
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ntpsec.org/pipermail/vc/attachments/20151122/f79b60d8/attachment.html>


More information about the vc mailing list