From owner-svn-src-all@FreeBSD.ORG Tue Jun 16 13:40:44 2009 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 515291065674; Tue, 16 Jun 2009 13:40:44 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 0FB638FC21; Tue, 16 Jun 2009 13:40:44 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 9948146B95; Tue, 16 Jun 2009 09:40:43 -0400 (EDT) Received: from jhbbsd.hudson-trading.com (unknown [209.249.190.8]) by bigwig.baldwin.cx (Postfix) with ESMTPA id 7DE078A073; Tue, 16 Jun 2009 09:40:42 -0400 (EDT) From: John Baldwin To: Bruce Evans Date: Tue, 16 Jun 2009 09:27:33 -0400 User-Agent: KMail/1.9.7 References: <200906101827.n5AIRFoR022115@svn.freebsd.org> <200906151328.32279.jhb@freebsd.org> <20090616091039.Q25629@delplex.bde.org> In-Reply-To: <20090616091039.Q25629@delplex.bde.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200906160927.34285.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Tue, 16 Jun 2009 09:40:42 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.5 required=4.2 tests=AWL,BAYES_00,RDNS_NONE autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r193941 - head/sys/netinet X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Jun 2009 13:40:45 -0000 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