From owner-svn-src-head@FreeBSD.ORG Sun Oct 27 17:29:38 2013 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id DDAFCFAF; Sun, 27 Oct 2013 17:29:37 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by mx1.freebsd.org (Postfix) with ESMTP id 6B4992232; Sun, 27 Oct 2013 17:29:37 +0000 (UTC) Received: from c122-106-156-23.carlnfd1.nsw.optusnet.com.au (c122-106-156-23.carlnfd1.nsw.optusnet.com.au [122.106.156.23]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 44DBB1041C1A; Mon, 28 Oct 2013 04:01:30 +1100 (EST) Date: Mon, 28 Oct 2013 04:01:29 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Ian Lepore Subject: Re: svn commit: r257200 - in head/sys/arm: allwinner allwinner/a20 arm at91 broadcom/bcm2835 econa freescale/imx include lpc mv rockchip samsung/exynos tegra ti ti/am335x ti/omap4 ti/twl versatile xili... In-Reply-To: <201310270134.r9R1YBbI037760@svn.freebsd.org> Message-ID: <20131028030951.V929@besplex.bde.org> References: <201310270134.r9R1YBbI037760@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=bpB1Wiqi c=1 sm=1 tr=0 a=ebeQFi2P/qHVC0Yw9JDJ4g==:117 a=PO7r1zJSAAAA:8 a=wN7JUwX51CgA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=HEFGpEVvdEcA:10 a=FAH0mBd8HFrLEcHYz4UA: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.14 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: Sun, 27 Oct 2013 17:29:38 -0000 On Sun, 27 Oct 2013, Ian Lepore wrote: > Log: > Remove #include from all the arm code that doesn't > really need it. That would be almost everywhere it was included. Add > it in a couple files that really do need it and were previously getting > it by accident via another header. Oops, I complained too soon about style bugs attached to these includes in another reply. Removing the includes also removes most of the collateral style bugs. How do you find the files that _really_ do need it? Nested pollution makes this difficult. tools/kerninclude barely works. When I tried to reduce nested pollution, I used a predecessor of tools/kerninclude that was more usable because it was more localized, but it got other things wrong. > Modified: head/sys/arm/include/undefined.h > ============================================================================== > --- head/sys/arm/include/undefined.h Sun Oct 27 00:51:46 2013 (r257199) > +++ head/sys/arm/include/undefined.h Sun Oct 27 01:34:10 2013 (r257200) > @@ -52,7 +52,9 @@ > > #include > > -typedef int (*undef_handler_t) (unsigned int, unsigned int, trapframe_t *, int); > +struct trapframe; > + > +typedef int (*undef_handler_t) (unsigned int, unsigned int, struct trapframe *, int); > > #define FP_COPROC 1 > #define FP_COPROC2 2 Other uses of the trapframe_t style bug reqiore more editing. This adds a style bug (extra blank line which separates the forward declaration from its one use). Old style bugs remaining in the main changed line include: - not using u_int (perhaps it should be foo_t) - gnu style parentheses - long line I don't like extensive use of typedefs to obfuscate function pointers, even without the style bug of declaring a typedef for the pointer and not a typedef for the function. We use typedefs for function pointers because the syntax for a function pointer is complicated, not primarily to hide the type. Here the pointer is used only a few times so repeating the declaration would be clearer. However, if the typedef were for the function then it could be used more. Now, most of the few uses are for a public function that seems to be never called: - install_coproc_handler() is declared in this header and its declaration uses the typedef - install_coproc_handler() is defined in arm/undefined.c and its definition uses the typedef - install_coproc_handler() seems to be never called. The installation function that is actually called is install_coproc_handler_static(). This was apparently originally a static internal for install_coproc_handler(). It is still named with 'static' and is still used internally but is now public and used externally. - a critical struct declaration in this header also uses the typedef. install_coproc_handler_static() takes a pointer to this struct as an arg so its callers don't need the typedef. Since the typedef is for a pointer and not a function, it cannot be used in forward declarations of handlers. Even if the typedef were for a function, C syntax wouldn't allow using it in definitions of handlers. So the typedef ends up being used once in used code and twice in unused code. Old style bugs visible in unchanged lines: - space instead of tab after #define. Bruce