From owner-freebsd-bugs@FreeBSD.ORG Tue Apr 29 08:00:30 2003 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id D2A1F37B401 for ; Tue, 29 Apr 2003 08:00:30 -0700 (PDT) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 542A243F93 for ; Tue, 29 Apr 2003 08:00:30 -0700 (PDT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.12.9/8.12.9) with ESMTP id h3TF0UUp037919 for ; Tue, 29 Apr 2003 08:00:30 -0700 (PDT) (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.12.9/8.12.9/Submit) id h3TF0UOe037918; Tue, 29 Apr 2003 08:00:30 -0700 (PDT) Date: Tue, 29 Apr 2003 08:00:30 -0700 (PDT) Message-Id: <200304291500.h3TF0UOe037918@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Bruce Evans Subject: Re: bin/51488: Compat patch: more(1) allowed filename to start with '+' X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list Reply-To: Bruce Evans List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Apr 2003 15:00:31 -0000 The following reply was made to PR bin/51488; it has been noted by GNATS. From: Bruce Evans To: Peter Pentchev Cc: freebsd-gnats-submit@freebsd.org Subject: Re: bin/51488: Compat patch: more(1) allowed filename to start with '+' Date: Wed, 30 Apr 2003 00:54:53 +1000 (EST) On Tue, 29 Apr 2003, Peter Pentchev wrote: > On Mon, Apr 28, 2003 at 02:44:04PM +0400, Dmitry Sivachenko wrote: > > -#define isoptstring(s) (((s)[0] == '-' || (s)[0] == '+') && (s)[1] != '\0') > > +#define isoptstring(s) more_mode ? (((s)[0] == '-') && (s)[1] != '\0') : \ > > + (((s)[0] == '-' || (s)[0] == '+') && (s)[1] != '\0') > Just a minor style/correctness comment: IMHO, the ?: conditional should > be enclosed in another set of parentheses. This is necessary for correctness (in macros that expand to an expression, the expression should have outer parethenthses, but it is not what KNF actually does for "?:" so it would be a style bug in most contexts. Unnecessary parentheses in macros are larger style bugs than in most places IMO, since macros need lots of parentheses for correctness and mixing unecessary ones with necessary ones makes the necessary ones hard to distinguish for human readers. In the above, the original version has minimal necessary parentheses including the outer ones, but the outer ones have mutated into unnecessary ones around each term in the "?:". KNF is more or less defined by what KNF code like Lite2's kern/*.c does, not by what style(9) says. In Lite2's kern/*.c, there are 51 lines with "?:". Approx. 12 of these lines have unnecessary parentheses. 2 of these parenthesize the expression like in "x = (y ? u : v)". 9 of them parenthesize the conditional like in "(x & y) ? u : v". One parenthesizes a double conditional "x ? y : (u ? v : w)". Unnecessary parentheses work worst for longer multiple conditionals like the latter. E.g., the idiomatic ladder: a ? b : c ? d : e ? f : g ? h : i ? j : k becomes: a ? b : (c ? d : (e ? f : (g ? h : (i ? j : k)))) where it is hard to unpeel the parentheses to see that nothing special is being done with them. This is like obfuscating "else if" ladders using "else { if ... }". Bruce