svn commit: r193941 - head/sys/netinet

John Baldwin jhb at freebsd.org
Tue Jun 16 13:40:45 UTC 2009


On Monday 15 June 2009 8:04:42 pm Bruce Evans wrote:
> On Mon, 15 Jun 2009, John Baldwin wrote:
> 
> > On Friday 12 June 2009 8:56:56 pm Bruce Evans wrote:
> >> On Fri, 12 Jun 2009, John Baldwin wrote:
> > ...
> >>> FWIW, the variables were signed before they were changed to unsigned and are now
> >>> back as signed again.  It just seems really odd to have the types not match
> >>> 'ticks' especially since many of the values are just cached copies of 'ticks'.
> >>
> >> No, they were originally u_long and stayed that way until you changed them
> >> (except possibly for newer variables).  The type of `ticks' is just wrong,
> >> and fixing that would take more work, but there is no reason to propagate
> >> this bug to new variables.  The original version is:
> >
> > Gah, I had misremembered the diffs, they were indeed unsigned.  Here is my
> > attempt at trying to fix things then based on my understanding so far:
> 
> This is about what I want.

Ok, I will commit in a bit with some fixes.

> > - I cast 'ticks' to u_int in the code to compute a new isn.  I'm not sure if
> >  this is needed.
> 
> This shouldn't be needed.

Ok, I've removed it.

> > --- //depot/projects/smpng/sys/netinet/tcp_timer.c	2009/04/14 19:06:19
> > +++ //depot/user/jhb/socket/netinet/tcp_timer.c	2009/06/15 17:22:13
> > @@ -418,8 +418,8 @@
> > 	 * backoff that we would use if retransmitting.
> > 	 */
> > 	if (tp->t_rxtshift == TCP_MAXRXTSHIFT &&
> > -	    ((ticks - tp->t_rcvtime) >= tcp_maxpersistidle ||
> > -	     (ticks - tp->t_rcvtime) >= TCP_REXMTVAL(tp) * tcp_totbackoff)) {
> > +	    (ticks - tp->t_rcvtime >= tcp_maxpersistidle ||
> > +	     ticks - tp->t_rcvtime >= TCP_REXMTVAL(tp) * tcp_totbackoff)) {
> > 		TCPSTAT_INC(tcps_persistdrop);
> > 		tp = tcp_drop(tp, ETIMEDOUT);
> > 		goto out;
> 
> Do we want to keep the non-KNF indentation (1 char extra to line things up)?

It is easier to read with it lined up and in this case I just went with the
existing style rather than reindenting.

> > --- //depot/projects/smpng/sys/netinet/tcp_var.h	2009/06/12 13:45:50
> > +++ //depot/user/jhb/socket/netinet/tcp_var.h	2009/06/15 17:22:13
> > @@ -139,12 +139,12 @@
> >
> > 	u_int	t_maxopd;		/* mss plus options */
> >
> > -	int	t_rcvtime;		/* inactivity time */
> > -	int	t_starttime;		/* time connection was established */
> > -	int	t_rtttime;		/* round trip time */
> > +	u_int	t_rcvtime;		/* inactivity time */
> > +	u_int	t_starttime;		/* time connection was established */
> > +	u_int	t_rtttime;		/* round trip time */
> 
> I would prefer the delta-times (like t_rtttime) to remain as ints.  Having
> everything of the same type is safer, and having everything of signed
> type allows better overflow checking (gcc -ftrapv (*)).  Whether ints are
> actually safer for r_tttime depend on how it is used.  Oops, I misread
> t_rtttime as being the rtt and a delta-time.  It seems to be actually
> an absolute time for the start of rtt measurement.  Its comment is thus
> now just wrong.

Ok, I've updated the comment instead and left it as u_int.

-- 
John Baldwin


More information about the svn-src-head mailing list