Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 20 Apr 2009 17:44:08 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Robert Watson <rwatson@FreeBSD.org>
Cc:        svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org
Subject:   Re: svn commit: r191292 - head/libexec/rtld-elf
Message-ID:  <20090420171843.A58020@delplex.bde.org>
In-Reply-To: <200904192303.n3JN3v9w023289@svn.freebsd.org>
References:  <200904192303.n3JN3v9w023289@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 19 Apr 2009, Robert Watson wrote:

> Log:
>  Explicitly include machine/param.h for CACHE_LINE_SIZE.
>
>  MFC after:	2 weeks
>
> Modified:
>  head/libexec/rtld-elf/rtld_lock.c

It is a style bug (unwarranted chumminess with the implementation) to
include (or otherwise refer to) machine/param.h directly in *.c.  This
is only the 5th instance of this style bug in all of /usr/src.  The
previous ones are:

% ./contrib/tcsh/sh.time.c:# include <machine/param.h>
% ./lib/libc/stdlib/malloc.c:#include <machine/param.h>
% ./sys/kern/subr_pcpu.c: *  - The platform sets the value of MAXCPU in <machine/param.h>.
% ./sys/mips/mips/stack_machdep.c:#include <machine/param.h>

Similarly in MI code that is not sys/param.h, except for 1 exception:

% ./gnu/lib/libstdc++/config.h:/* Define to 1 if you have the <machine/param.h> header file. */
% ./contrib/libstdc++/libmath/mathconf.h:#     include <machine/param.h>
% ./contrib/binutils/bfd/hosts/decstation.h:/* Hopefully this should include either machine/param.h (Ultrix) or
% ./contrib/binutils/bfd/hosts/mipsbsd.h:#include <machine/param.h>
% ./contrib/binutils/bfd/hosts/i386bsd.h:#include <machine/param.h>
% ./contrib/gdb/gdb/config/mips/xm-mips.h:// OBSOLETE #include <machine/param.h>

Contribed code certainly shouldn't know the implementation details.  It
might have ifdefs for this, with machine/param.h not being an implementation
detail on some systems, but the includes in *bsd.h are probably wrong since
bsd is not such a system.

% ./sys/dev/drm/drmP.h:#include <machine/param.h>
% ./sys/i386/include/xen/xen-os.h:#include <machine/param.h>

Probably just wrong.

% ./sys/sys/param.h:#include <machine/param.h>

This is the public interface.

% ./sys/sys/socket.h:#include <machine/param.h>

This one is correct.  It needs to include <machine/param.h> specially to
get a small subset to avoid namespace pollution in the standard header
sys/socket.h.  This reason doesn't apply in any other cases.  BTW,
the #defines of __HAVE_ACPI and __PCI_REROUTE_INTERRUPT are unsorted
more than most, at least in i386/include/param.h -- they are unsorted
outside of the normal _FOO_H_ include guard.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090420171843.A58020>