From owner-freebsd-arch@FreeBSD.ORG Fri Nov 30 04:18:20 2007 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 491B216A419 for ; Fri, 30 Nov 2007 04:18:20 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from fallbackmx10.syd.optusnet.com.au (fallbackmx10.syd.optusnet.com.au [211.29.132.251]) by mx1.freebsd.org (Postfix) with ESMTP id D715513C461 for ; Fri, 30 Nov 2007 04:18:18 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail05.syd.optusnet.com.au (mail05.syd.optusnet.com.au [211.29.132.186]) by fallbackmx10.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id lATN59Ji004420 for ; Fri, 30 Nov 2007 10:05:11 +1100 Received: from besplex.bde.org (c211-30-219-213.carlnfd3.nsw.optusnet.com.au [211.30.219.213]) by mail05.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id lATN4svY011397 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 30 Nov 2007 10:04:58 +1100 Date: Fri, 30 Nov 2007 10:04:54 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: "M. Warner Losh" In-Reply-To: <20071129.084108.-713549098.imp@bsdimp.com> Message-ID: <20071130094705.E6718@besplex.bde.org> References: <20071128.151021.709401576.imp@bsdimp.com> <86lk8hhzs0.fsf@ds4.des.no> <20071129.084108.-713549098.imp@bsdimp.com> MIME-Version: 1.0 Content-Type: MULTIPART/MIXED; BOUNDARY="0-1458219190-1196377494=:6718" Cc: des@des.no, arch@freebsd.org Subject: Re: Code review request: small optimization to localtime.c X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Nov 2007 04:18:20 -0000 This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --0-1458219190-1196377494=:6718 Content-Type: TEXT/PLAIN; charset=X-UNKNOWN; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE On Thu, 29 Nov 2007, M. Warner Losh wrote: > In message: <86lk8hhzs0.fsf@ds4.des.no> > Dag-Erling_Sm=F8rgrav writes: > : "M. Warner Losh" writes: > : > Please find enclosed some small optimizations. [...] > : > : almost completely unrelated, but while you're at it: > : > : > =09if (__isthreaded !=3D 0) { > : > : __isthreaded is clearly (by its name) a predicate, comparing it > : explicitly to 0 is redundant and disrupts my flow of thought when > : reading the code. Instead of just reading "if is threaded", I have to > : take a second to parse the expression and check which way the compariso= n > : goes. > : > : We already have a policy (unwritten as far as I know) of using explicit > : comparisons for variables which are not clearly predicates, can we also > : have one of *not* using explicit comparisons for those that are? And > : document both cases in style(9)? > > True, but very Brucian in the nature of the comment: I didn't change > this in existing code. :-) KNF rules are sort of the opposite in some respects -- -- "if ((flags & MASK) !=3D 0)", which is like the above, is slightly more normal than "if (flags & MASK)".=20 -- The unary "!" operator is rarely used. "if (!isfoo)" and "if (!(flags & MASK))" are not normal. Anyway, there is too much existing code with bad style to change. I draw the line (for non-booleans) between !error and !strcmp(). Bruce --0-1458219190-1196377494=:6718--