Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 19 Oct 2004 22:13:34 +1000 (EST)
From:      Bruce Evans <bde@zeta.org.au>
To:        "David O'Brien" <obrien@freebsd.org>
Cc:        cvs-all@freebsd.org
Subject:   Re: cvs commit: src/lib/libc/i386/net htonl.S ntohl.S
Message-ID:  <20041019211832.O1552@epsplex.bde.org>
In-Reply-To: <20041018190044.GA89411@hub.freebsd.org>
References:  <200410181719.i9IHJa9l097436@repoman.freebsd.org> <20041018173516.GB89681@ip.net.ua> <20041018174511.GA6079@dragon.nuxi.com> <20041018180319.GD89681@ip.net.ua> <20041018190044.GA89411@hub.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 18 Oct 2004, David O'Brien wrote:

> On Mon, Oct 18, 2004 at 09:03:20PM +0300, Ruslan Ermilov wrote:
> > You can check for the CPUTYPE="i386" for ${MACHINE_ARCH} == "i386"
> > in libc/Makefile, and set I386_CPU in CFLAGS in this case.
>
> Taking
>   Revision  Changes    Path
>   1.10      +4 -0      src/lib/libc/i386/net/htonl.S
>   1.10      +4 -0      src/lib/libc/i386/net/ntohl.S
>
> specifically what patch do you suggest that we used in cases like this
> rather than #ifdef I386_CPU".  And how to use it?  That is one in which
> someone checks out the source and wants to build a userland that is
> usable on an I386 machine.

The null patch (to makefiles and *nl.S rev.1.9) should be used in cases
like this.  The extern interfaces to htonl() and ntohl() are little more
than rarely used compatibility cruft.  Such interfaces should be just
be left compatible and not optimized, especially when the usual
macro/inline interfaces remain unoptimized (*).

In all of my slightly reduced version of the world, the extern interfaces
are used in only 2 places, and both of those uses are bugs:

(1) libsmb's smb_lib.h has macros which reference htonl() and ntohl(), but
    it doesn't include any of the headers that define or declare these
    functions.  In RELENG_4 these functions were normally obtained as
    macros via namespace pollution in <sys/types.h>, but the pollution
    has been fixed so that in -current these functions are only (?)
    defined in Standard places (<arpa/inet.h> and <netinet/in.h>).  So
    smb_lib.h is broken unless the .c source file happens to include
    one of the standard headers.  In the world, only libsmb's mbuf.c
    seems to not have enough includes for smb_lib.h to work.  The result
    is a single reference to htonl() in mbuf.*o.

(2) libc's a.out.h has similar bugs.  It includes <sys/endian.h> mainly
    so that the endianness macros used in <sys/imgact_aout.h> are
    defined, but <sys/imgact_aout.h> references htonl() and ntohl()
    in some places and <sys/endian.h> doesn't define these interfaces
    (it only defines non-networking ones).  In the world, only btxld's
    btxld.c seems to be affected by this.  The result is a single
    reference to ntohl() in btxld.o (but many references to ntohl()
    in cpp output).

    This bug seems to be just bitrot in <sys/imgact_aout.h> -- it uses
    the newer htole32() and le32toh() interfaces in many places but
    still uses htonl() and ntohl() in places that haven't been touched
    recently.

(*) From the original message in this thread:

%   Modified files:
%     lib/libc/i386/net    htonl.S ntohl.S
%   Log:
%   Sync with sys/i386/include/endian.h: use the single instruction 'bswap'.

This doesn't actually sync with sys/i386/include/endian.h, since it uses
a simple I386_CPU ifdef but endian.h uses a much larger ifdef that causes
the bswap instruction to never be used outside the kernel in the usual
(non-extern) case.  From endian.h:

% #if defined(_KERNEL) && (defined(I486_CPU) || defined(I586_CPU) || defined(I686_CPU)) && !defined(I386_CPU)
%
% #if defined(_KERNEL) && (defined(I486_CPU) || defined(I586_CPU) || defined(I686_CPU)) && !defined(I386_CPU)
%
% #define __byte_swap_int_var(x) \
% __extension__ ({ register __uint32_t __X = (x); \
%    __asm ("bswap %0" : "+r" (__X)); \
%    __X; })
% #else
%
% #define __byte_swap_int_var(x) \
% __extension__ ({ register __uint32_t __X = (x); \
%    __asm ("xchgb %h0, %b0\n\trorl $16, %0\n\txchgb %h0, %b0" \
%        : "+q" (__X)); \
%    __X; })
% #endif

So the only effect of this commit is to break i386 support for:
- libsmb and btxld in the world
- anything in the world that I missed
- any packages or ports that are affected by the bugs in the libsmb or
  a.out.h or have their own wrong includes.  (smb_lib.h is internal so
  it shouldn't affect ports.  Ports and not just binary packages are
  affected because they won't know to define I386_CPU.)
- any programs that intentially avoid using the macro versions.  (There
  seem to be none of these in the world, so if there are any then they
  won't know to define I386_CPU.)

Bruce



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