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...
Bruce Evans
brde at optusnet.com.au
Fri Dec 3 09:45:16 UTC 2010
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 <sys/param.h>
> +#include <sys/systm.h>
> +
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 <sys/types.h> instead of
<sys/param.h>, or <sys/param.h> without <sys/systm.h>, or include
<sys/systm.h> after other headers, may already be broken, since
KASSERT() is declared in <sys/systm.h>, but it may be used in other
header (like this one now). KASSERT() should probably be declared in
<sys/param.h> or even in <sys/cdefs.h>. That gives more pollution there
but less overall.)
> #include <machine/_bus.h>
> #include <machine/cpufunc.h>
Including <machine/_bus.h> is correct (_bus.h exist to avoid namespace
pollution that is about 1000 times smaller than now here), but including
<machine/cpufunc.h> 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 <machine/cpufunc.h> is
standard namespace pollution in <sys/systm.h>.
>
> @@ -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
More information about the svn-src-all
mailing list