From owner-svn-src-head@FreeBSD.ORG Fri Dec 3 09:45:16 2010 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A25DF10656AA; Fri, 3 Dec 2010 09:45:16 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail02.syd.optusnet.com.au (mail02.syd.optusnet.com.au [211.29.132.183]) by mx1.freebsd.org (Postfix) with ESMTP id 3ADCC8FC12; Fri, 3 Dec 2010 09:45:15 +0000 (UTC) Received: from c211-30-187-94.carlnfd1.nsw.optusnet.com.au (c211-30-187-94.carlnfd1.nsw.optusnet.com.au [211.30.187.94]) by mail02.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id oB39jCUl020765 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 3 Dec 2010 20:45:13 +1100 Date: Fri, 3 Dec 2010 20:45:12 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Bruce Cran In-Reply-To: <201012022219.oB2MJUx5031472@svn.freebsd.org> Message-ID: <20101203201705.O2228@besplex.bde.org> References: <201012022219.oB2MJUx5031472@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r216134 - in head: share/man/man9 sys/amd64/include sys/arm/include sys/i386/include sys/ia64/include sys/mips/include sys/pc98/include sys/powerpc/include sys/sparc64/include sys/sun4v... X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 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, 03 Dec 2010 09:45:16 -0000 On Thu, 2 Dec 2010, Bruce Cran wrote: > Log: > Disallow passing in a count of zero bytes to the bus_space(9) functions. > > Passing a count of zero on i386 and amd64 for [I386|AMD64]_BUS_SPACE_MEM > causes a crash/hang since the 'loop' instruction decrements the counter > before checking if it's zero. > > PR: kern/80980 > Discussed with: jhb > ... > Modified: head/sys/amd64/include/bus.h > ============================================================================== > --- head/sys/amd64/include/bus.h Thu Dec 2 22:00:57 2010 (r216133) > +++ head/sys/amd64/include/bus.h Thu Dec 2 22:19:30 2010 (r216134) > @@ -104,6 +104,9 @@ > #ifndef _AMD64_BUS_H_ > #define _AMD64_BUS_H_ > > +#include > +#include > + This is massive namespace pollution. Most kernel .c files should include these first, and most already do. (Ones that try to be smart and only include instead of , or without , or include after other headers, may already be broken, since KASSERT() is declared in , but it may be used in other header (like this one now). KASSERT() should probably be declared in or even in . That gives more pollution there but less overall.) > #include > #include Including is correct (_bus.h exist to avoid namespace pollution that is about 1000 times smaller than now here), but including is older namespace pollution/historical mislayering (we only need i/o functions from cpufunc.h, and they should be declared here directly). Now it has no effect, since is standard namespace pollution in . > > @@ -268,7 +271,7 @@ static __inline void > bus_space_read_multi_1(bus_space_tag_t tag, bus_space_handle_t bsh, > bus_size_t offset, u_int8_t *addr, size_t count) > { > - > + KASSERT(count != 0, ("%s: count == 0", __func__)); > if (tag == AMD64_BUS_SPACE_IO) > insb(bsh + offset, addr, count); > else { KASSERT() in little inline functions gives a lot of bloat for such an unlikely error. Stupid callers can still pass any garbage count except 0. The function name of a leaf function is not very interesting. In some of the other bus.h's, the caller's name is available since the interface is a macro, but there __func__ (which should only be used in macros) is not used, apparently since it would give a name that is useful but inconsistent with arches that don't use a macro. Bruce