svn commit: r274088 - head/sys/kern

[Bruce Evans]

On Tue, 4 Nov 2014, Ian Lepore wrote:

> On Wed, 2014-11-05 at 07:46 +1100, Bruce Evans wrote:
>> ...
>> _ALIGNBYTES should probably have type u_long to begin with to reduce the
>> magic.
>> ...
>
> So bottom line, define _ALIGNBYTES as (8UL - 1) to minimize the magic?

8UL on 64-bit arches and 8U on 32-bit arches.

The second is required to avoid surprises and not break the comment which
says that _ALIGN() has type unsigned int.  Except the comment is often
broken.  All cases are now broken in some way, including some that were
correct except for style bugs in FreeBSD-[4-5] :-(,

The less-broken cases use 'unsigned long' or 'unsigned int' in the
definition of _ALIGN() so as to avoid namespace pollution.  This
type should be identical to uintptr_t, and must be equivalent to
uintptr_t.  The documentation of the type in the comment should
match this.

The current (and some previous) brokenness is:

amd64:
This was broken in FreeBSD-[5-8] by spelling uintptr_t as u_long in
the code and comments.  Using u_long gives namespace pollution and
thus defeats the reason for existence of _align.h, but FreeBSD-[5-8]
didn't use _align.h.  The namespace pollution was moved by merging
with the i386 _align.h.  The u_long in the code is now spelled
uintptr_t.  The long in the code is now spelled register_t.  The u_long
in the comment is now just wrong for amd64.  It is now misspelled
unsigned int, after copying the comment from i386 where it was correct.

arm:
Correct, except for the style bug of spelling the type verbosely as
unsigned int in the comment.  Only 32 bits is supported, and uintptr_t
is always uint32_t == unsigned, so there are no complications.

i386, pc98:
This was correct in FreeBSD[4-8] except for the same style bug as arm.
(I changed tyhe comment to say uintptr_t in my version.  This spelling
doesn't cause any namespace problems when it is in a comment.)  i386
now uses the same code as amd64.  The type in the comment remains
correct, but the types in the code give namespace pollution.

mips
This still uses the old alpha/NetBSD (arch) code and spells the type as
u_long in the comment and the code.  This is more broken than before,
since u_long is incompatible with the type of uintptr_t in the 32-bit
case.  uintptr_t == uint32_t == plain unsigned then.  It gives the same
namespace pollution as on old amd64 (which was also closer to alpha than
to i386). More details on the pollution: u_long is not even defined when
in the case where _align.h does something useful (when sys/socket.h is
compiled with options for strict POSIX conformance).

powerpc:
Same as amd64.  More details on the namsespace pollution: types ending in
_t are in the POSIX namespace in some cases, so uintptr_t and register_t
are not quite as polluting as u_long.

sparc64:
this uses u_long in the code but unsigned int in the comment.  It is like
old alpha except for the wrong comment.

ia64:
Now dead.  In FreeBSD-10, it was like old alpha except for having
ALIGNBYTES = 15 instead of 7.  This signed type gives the magic previously
discussed.  Another archaism from old alpha is spelling
'& ~ALIGNBYTES' as '&~ ALIGNBYTES'.

All arches except mips and powerpc now avoid the signed magic by spelling
_ALIGNBYTES using sizeof(sizeof(foo_t)).  sizeof() gives type size_t without
spelling it as such, thus has no namespace problems in itself, and also
gives the correct type (u_int or u_long) on all supported arches.  However,
using foo_t gives namespace problems.

The namespace problems can also be avoided by including <machine/_types.h>
and using __foo_t instead of foo_t.  I don't like that much, though its
overhead is small since almost all headers end up including
<machine/_types.h>.

Using literal constants also has the advantage of making _ALIGNBYTES usable
in cpp expressions.  Since it is an implementation detail, this is not
very important.

Bruce