From owner-freebsd-hackers@FreeBSD.ORG Fri May 1 08:30:27 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 A91F5106564A for ; Fri, 1 May 2009 08:30:27 +0000 (UTC) (envelope-from julian@elischer.org) Received: from outC.internet-mail-service.net (outc.internet-mail-service.net [216.240.47.226]) by mx1.freebsd.org (Postfix) with ESMTP id 8AF1E8FC17 for ; Fri, 1 May 2009 08:30:27 +0000 (UTC) (envelope-from julian@elischer.org) Received: from idiom.com (mx0.idiom.com [216.240.32.160]) by out.internet-mail-service.net (Postfix) with ESMTP id 3E2D9C055E; Fri, 1 May 2009 01:30:27 -0700 (PDT) X-Client-Authorized: MaGic Cook1e X-Client-Authorized: MaGic Cook1e X-Client-Authorized: MaGic Cook1e Received: from julian-mac.elischer.org (home.elischer.org [216.240.48.38]) by idiom.com (Postfix) with ESMTP id B6EA22D60F9; Fri, 1 May 2009 01:30:26 -0700 (PDT) Message-ID: <49FAB322.9030103@elischer.org> Date: Fri, 01 May 2009 01:30:26 -0700 From: Julian Elischer User-Agent: Thunderbird 2.0.0.21 (Macintosh/20090302) MIME-Version: 1.0 To: Christoph Mallon References: <49F4070C.2000108@gmx.de> <20090428114754.GB89235@server.vk2pj.dyndns.org> <20090430.090226.1569754707.imp@bsdimp.com> <49FA8D73.6040207@gmx.de> In-Reply-To: <49FA8D73.6040207@gmx.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 8bit Cc: freebsd-hackers@FreeBSD.org 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: Fri, 01 May 2009 08:30:27 -0000 As an old-fart I have found many cases where what I thought was a silly style rule, turned out to save my work in some way. Christoph Mallon wrote: >> >> >> 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; Well more natural for you, but not necessarily for everyone, and NOT the same as what is there now, as you noticed. >> >> 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 > > You're mistaken, this is perfectly legal C. See ISO/IEC 9899:1999 (E) > §6.8.2:1. In short: you can mix statements and declarations. now, but not all C compilers are C99 and a lot of FreeBSD code is taken and run in other situations. There is FreeBSD code in all sorts of environments, not all of which have new compilers. Besides which I'd vote against that just on principle that it breaks current style too much, and it makes it hard to find where things are declared. > >> : [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. I find return values being parenthesised is easier to for me to read. I'd vote against his too >> >> : [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. This is not new. It's already policy to remove them whenever you are working in the area. it improves code error checking.. >> >> : [ 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. > > Would you commit these three proposals? > >> : >> +.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. it does however make it harder to find stuff in gdb As a general rule, one of the things that is good about BSD code is that it all looks about the same. This makes it easier to read, once you have got used to it. changing it for no reason except "I felt like it" will and I think should, meet stiff opposition.