From owner-svn-src-head@FreeBSD.ORG Fri May 2 08:34:54 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id AE618F36; Fri, 2 May 2014 08:34:54 +0000 (UTC) Received: from mail108.syd.optusnet.com.au (mail108.syd.optusnet.com.au [211.29.132.59]) by mx1.freebsd.org (Postfix) with ESMTP id 401FD19D5; Fri, 2 May 2014 08:34:54 +0000 (UTC) Received: from c122-106-147-133.carlnfd1.nsw.optusnet.com.au (c122-106-147-133.carlnfd1.nsw.optusnet.com.au [122.106.147.133]) by mail108.syd.optusnet.com.au (Postfix) with ESMTPS id 309461A4D6B; Fri, 2 May 2014 18:34:47 +1000 (EST) Date: Fri, 2 May 2014 18:34:40 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Luiz Otavio O Souza Subject: Re: svn commit: r265191 - head/sys/dev/gpio In-Reply-To: <201405011409.s41E9mun075016@svn.freebsd.org> Message-ID: <20140502180115.I1337@besplex.bde.org> References: <201405011409.s41E9mun075016@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=U6SrU4bu c=1 sm=1 tr=0 a=7NqvjVvQucbO2RlWB8PEog==:117 a=PO7r1zJSAAAA:8 a=WU5SAaDETzEA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=YRN3L0-C4pmnSk9kEeAA:9 a=CjuIK1q_8ugA:10 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 02 May 2014 08:34:54 -0000 On Thu, 1 May 2014, Luiz Otavio O Souza wrote: > Log: > Remove unnecessary headers. Sort out the headers. Add a missing header on > ofw_gpiobus.c (it was working because of sys/libkern.h). Do you use /usr/src/tools/tools/kerninclude/kerninclude.sh to find the unused includes? There are many false positives and negatives which are hard to find without a lot of testing. kerninclude.sh automates some of the testing. I think it is rarely used and has rotted. A full universe build is probably required, but kerninclude.sh is i386-centric and only tests LINT, GENERIC and GENERIC98 (GENERIC98 last existed in the source tree in FreeBSD-3, but the script creates it by copying GENERIC and building with pc98 options). I prefer my unusedinc script. It is easier to run 1 on file at a time, but does less hacking to reduce the false positives and negatives. > Modified: head/sys/dev/gpio/gpiobus.c > ============================================================================== > --- head/sys/dev/gpio/gpiobus.c Thu May 1 14:08:19 2014 (r265190) > +++ head/sys/dev/gpio/gpiobus.c Thu May 1 14:09:47 2014 (r265191) > @@ -28,21 +28,16 @@ > __FBSDID("$FreeBSD$"); > > #include > -#include This unsorts the includes. must be included after , and should be always included, since other headers may depend on it (for things like KASSERT() in inline functions). Some broken headers include it nested. This makes it difficult to tell whether it is used. Some other headers that don't do this may compile accidentally because they are included after the polluted ones. > +#include > +#include > +#include > #include > #include > -#include > -#include is probably used. It is included nested in many headers, and this is not considered pollution, unlike for , but it makes it very hard to determine if is included as needed in other headers and .c files. kerninclude.sh attempts to determine if headers like are needed by doing things like replacing it by an empty header in some contexts. I suspect this doesn't handle the full complications. Ideally, the include of in a .c file should be explicit iff the .c files uses any queue macro. > -#include > +#include > #include is certainly not needed. It is standard pollution in , and it is a style bug to not depend on that. It is also usually a style bug (in kernel code) to include and not include . Many things depend on the standard pollution, or might be changed to depend on it. > Modified: head/sys/dev/gpio/ofw_gpiobus.c > ============================================================================== > --- head/sys/dev/gpio/ofw_gpiobus.c Thu May 1 14:08:19 2014 (r265190) > +++ head/sys/dev/gpio/ofw_gpiobus.c Thu May 1 14:09:47 2014 (r265191) > @@ -33,17 +33,13 @@ __FBSDID("$FreeBSD$"); > #include > #include > #include > -#include Correct. is standard pollution in , and it is a style bug to include it directly. > -#include > #include > -#include > +#include Not using mutexes may indicate missing locking. I think this works because almost everything in new-bus is Giant-locked and Giant locking hides its own details very well. Elsewhere, there are lots of polluting nested includes of and (not even of and which make it very unclear whether explicit includes of these are needed. The worst cases are in , , , , and . Only some of these are relatively easy to fix by including in the headers. Bruce