From owner-freebsd-hackers@FreeBSD.ORG Thu Apr 30 15:07:13 2009 Return-Path: Delivered-To: freebsd-hackers@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 30483106566B for ; Thu, 30 Apr 2009 15:07:13 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (bsdimp.com [199.45.160.85]) by mx1.freebsd.org (Postfix) with ESMTP id D17898FC0A for ; Thu, 30 Apr 2009 15:07:12 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from localhost (localhost [127.0.0.1]) by harmony.bsdimp.com (8.14.2/8.14.1) with ESMTP id n3UF2ORH017382; Thu, 30 Apr 2009 09:02:25 -0600 (MDT) (envelope-from imp@bsdimp.com) Date: Thu, 30 Apr 2009 09:02:26 -0600 (MDT) Message-Id: <20090430.090226.1569754707.imp@bsdimp.com> To: peterjeremy@optushome.com.au From: "M. Warner Losh" In-Reply-To: <20090428114754.GB89235@server.vk2pj.dyndns.org> References: <49F4070C.2000108@gmx.de> <20090428114754.GB89235@server.vk2pj.dyndns.org> X-Mailer: Mew version 5.2 on Emacs 21.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: freebsd-hackers@FreeBSD.org, christoph.mallon@gmx.de Subject: Re: C99: Suggestions for style(9) X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Apr 2009 15:07:13 -0000 In message: <20090428114754.GB89235@server.vk2pj.dyndns.org> Peter Jeremy writes: : >Maybe using all of these changes is a bit too big change at once, but : >I'd like some of them applied to style(9). So, please consider the : >proposed changes individually and not a as a all-or-nothing package. : : One area you do not address is code consistency. Currently, the : FreeBSD kernel (and, to a lesser extent, userland) are mostly style(9) : compliant - ie the entire codebase is mostly written in the same : style. Whilst you may not like it (and I don't know that anyone : completely agrees with everything in style(9)), it does mean that : the code is consistent. : : Changing style(9) in a way that is not consistent with current code : means that either existing code must be brought into line with the : new standard - which incurs a one-off cost - or the code base becomes : split into "old" and "new" style - incurring an ongoing maintenance : cost as maintainers switch between styles. Both approaches incur a : risk of introducing new bugs. : : Note that I'm not saying that style(9) can never be changed, I'm just : saying that any change _will_ incur a cost and the cost as well as : the potential benefits need to be considered. This is my biggest worry about changing style(9) quickly. We don't want needless churn in the tree for just style changes since it makes it harder to track bug fixes into the past. : [Reduce declaration scope as far as possible, initialise variables where : they are declared, sort by name] : : Whilst my personal preference is for the style suggested by Christoph : (and I generally agree with the points he makes), this is also the : change that incurs the biggest stylistic change. This is not a change : that can be practically retrofitted to existing code and therefore its : implementation would result in a mixture of code styles - increasing : the risk of bugs due to confusion as to which style was being used. I : am not yet sure whether the benefits outweigh the costs, This is the biggest one, and I think it may be too soon. Also, we need to be careful on the initialization side of things because we currently have a lot of code that looks like: struct foo *fp; struct bar *bp; fp = get_foo(); if (!fp) return; bp = fp->bp; this can't easily be translated to the more natural: struct foo *fp = get_foo(); struct bar *bp = fp->bp; since really you'd want to write: struct foo *fp = get_foo(); if (!fp) return; struct bar *bp = fp->bp; which isn't legal in 'C'. However, we have enough where this isn't the case that it should be allowed. We should separate the initialization part of this from the scope part of this too. The initialization part is already leaking into the code, while instances of the scope restriction is rarer. : [Don't parenthesize return values] : >Removed, because it does not improve maintainability in any way. : : This change could be made and tested mechanically. But there is : no justification for making it - stating that the existing rule : is no better than the proposed rule is no reason to change. I'm fine with this. : [No K&R declarations] : >Removed, because there is no need to use them anymore. : : Whilst this is a change that could be performed mechanically, there : are some gotchas relating to type promotion (as you point out). : The kernel already contains a mixture of ANSI & K&R declarations and : style(9) recommends using ANSI. IMHO, there is no need to make this : change until all K&R code is removed from FreeBSD. I'm fine with this change. : [ Don't insert an empty line if the function has no local variables.] : : This change could be made and tested mechanically. IMHO, this change : has neglible risk and could be safely implemented. I'm fine with this change. : >> +.Sh LOCAL VARIABLES : : >Last, but definitely not least, I added this paragraph about the use of : >local variables. This is to clarify, how today's compilers handle : >unaliased local variables. : : Every version of gcc that FreeBSD has ever used would do this for : primitive types when optimisation was enabled. This approach can : become expensive in stack (and this is an issue for kernel code) when : using non-primitive types or when optimisation is not enabled (though : I'm not sure if this is supported). Assuming that gcc (and icc and : clang) behaves as stated in all supported optimisation modes, this : change would appear to be quite safe to make. Agreed, in general. We don't want to optimize our code style based on what one compiler does, perhaps on x86. Warner