From owner-svn-src-head@FreeBSD.ORG Fri Jun 1 16:04:26 2012 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 84C8B1065672; Fri, 1 Jun 2012 16:04:26 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail27.syd.optusnet.com.au (mail27.syd.optusnet.com.au [211.29.133.168]) by mx1.freebsd.org (Postfix) with ESMTP id F14148FC0A; Fri, 1 Jun 2012 16:04:25 +0000 (UTC) Received: from c122-106-171-232.carlnfd1.nsw.optusnet.com.au (c122-106-171-232.carlnfd1.nsw.optusnet.com.au [122.106.171.232]) by mail27.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id q51G4GfG021209 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 2 Jun 2012 02:04:18 +1000 Date: Sat, 2 Jun 2012 02:04:16 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: John Baldwin In-Reply-To: <201206011024.49122.jhb@freebsd.org> Message-ID: <20120602015640.P3206@besplex.bde.org> References: <201206010423.q514NKtf083043@svn.freebsd.org> <20120601150242.V1226@besplex.bde.org> <201206011024.49122.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Eitan Adler , Bruce Evans Subject: Re: svn commit: r236377 - head/sys/dev/vxge/vxgehal X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Jun 2012 16:04:26 -0000 On Fri, 1 Jun 2012, John Baldwin wrote: > On Friday, June 01, 2012 2:23:42 am Eitan Adler wrote: >> On 31 May 2012 22:13, Bruce Evans wrote: >>> This seems to change a style by (excessive parentheses for a normal >>> equality test) into logic bug (assignment of dtrh instead of compariing >>> with it). >> >> intentional - perhaps my commit message was poorly worded. >> >> The comment above says >> 283 /* >> 284 * restore a previously allocated dtrh at current offset and update >> 285 * the available reserve length accordingly. If dtrh is null just >> 286 * update the reserve length, only >> 287 */ >> >> and gnn confirmed that the patch as committed is correct. Oops. > This is why I personally loathe assignment side effects in boolean expressions > for control flow. I tend to write this sort of thing instead as: > > channel->dtr_arr[dtr_index].dtr = dtrh; > if (dtrh != NULL) { Except here you would have written: channel->dtr_arr[dtr_index].dtr = dtrh; if (dtrh == NULL) return; to avoid a large compound statement for the null case. Bruce