From owner-freebsd-arch@FreeBSD.ORG Thu Mar 10 18:35:21 2011 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8D4CF1065758 for ; Thu, 10 Mar 2011 18:35:21 +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 3D03E8FC14 for ; Thu, 10 Mar 2011 18:35:21 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by harmony.bsdimp.com (8.14.3/8.14.1) with ESMTP id p2AIO97H087193 for ; Thu, 10 Mar 2011 11:24:11 -0700 (MST) (envelope-from imp@bsdimp.com) Message-ID: <4D791749.9020803@bsdimp.com> Date: Thu, 10 Mar 2011 11:24:09 -0700 From: Warner Losh User-Agent: Mozilla/5.0 (X11; U; FreeBSD amd64; en-US; rv:1.9.2.13) Gecko/20101211 Thunderbird/3.1.7 MIME-Version: 1.0 To: freebsd-arch@freebsd.org References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: style(9) rules for nested includes 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: Thu, 10 Mar 2011 18:35:21 -0000 On 03/10/2011 10:17, mdf@freebsd.org wrote: > I recall a recent discussion/PR about nested includes in the context > of and being a few of the only ones > allowed. However, I don't see anything in style(9) about this. > > One rule for nested includes I've heard (that FreeBSD doesn't use, but > just mentioning it for variety) is that every header file should > compile on its own; that is, a .c file with nothing but #include > should compile without errors. This rule also makes some > sense with the FreeBSD style convention of alphabetizing includes; > otherwise it's kinda just happenstance that e.g. comes > alphabetically before,, and > , and barely before. We've explicitly not done that to date. In the past, this would cause extra I/O and processing that would slow down compile times by a measurable amount. These days, I think you'd be hard pressed to even measure the effect. However, if we adopted this rule, you tend to get n^2 behavior because each layer of the onion redundantly would be including the inner onion... A survey of the include files shows uneven application of rules. Some require additional includes before them (the most common being sys/param.h or sys/types.h), some include some prereques, etc. There's no hard and fast rule that will describe the current state of affairs. > Now we come to the reason I ask. I'm working on a patch to change the > static sysctl code to use the standard SYSININT/SYSUNINIT code rather > than have special treatment in kern_linker.c, but to do this I need to > either change quite a few places that include, or > include instead of in sysctl.h, as > the SI_SUB_SYSCTLS value isn't visible otherwise. > > This, though, shows a bug in, where it uses the > constant MAXPATHLEN in the extern declaration of kernelname[] (which > itself I think is not standard C, IIRC, but there are several uses of > sizeof(kernelname) that don't work otherwise). So then > needs to build. It is cascade problems like this which both militate for the change and against it. For the change because it causes less churn (in theory). Against the change because you tend to accumulate lots of things that used to be required, but are no longer, but which consumers come to bogusly depend on, making it hard to detangle the thicket. An interesting number of include files assume that param.h has already been included, and avoids the extra parsing overhead that skipping param.h N times would cause. It likely is better to fix the 6 places in the kernel where sizeof is used than to redundantly include param.h. Also, while you're there, fix the strncpy to a strlcpy: ./kern/kern_mib.c: kernelname, sizeof kernelname, "Name of kernel file booted"); ./sun4v/sun4v/machdep.c: strlcpy(kernelname, env, sizeof(kernelname)); ./sparc64/sparc64/machdep.c: strlcpy(kernelname, env, sizeof(kernelname)); ./powerpc/aim/machdep.c: strlcpy(kernelname, env, sizeof(kernelname)); ./amd64/amd64/machdep.c: strlcpy(kernelname, env, sizeof(kernelname)); ./ia64/ia64/machdep.c: strncpy(kernelname, p, sizeof(kernelname) - 1); > So, which files are okay to include in an include file? Does someone > (I'll do it if we can agree on a list) want to put this in style(9)? I doubt it would be wise to make a blanket statement like all headers must be self-contained. There's name-space contamination issues to worry about, etc. Warner