Index | Thread | Search

From:
Christian Weisgerber <naddy@mips.inka.de>
Subject:
Re: Cruft: drop htonl() etc. from libc
To:
tech@openbsd.org
Date:
Fri, 12 Apr 2024 15:42:12 +0200

Download raw body.

Thread
Claudio Jeker:

> >  u_int16_t
> >  ntohs(u_int16_t x)
> >  {
> > -#if BYTE_ORDER == LITTLE_ENDIAN
> > -	u_char *s = (u_char *) &x;
> > -	return (u_int16_t)(s[0] << 8 | s[1]);
> > -#else
> > -	return x;
> > -#endif
> > +	return __htobe16(x);
> 
> This looks the wrong way around. ntohs() is be16toh(). Same for ntohl
> above. While it does not matter for our cases I think we should use the
> right conversions here.

I was going by the #defines in <endian.h>, but sure, it makes sense
to use the logically appropriate public functions.  While there,
I've also switched to the POSIX types.

Here's the updated diff, without the file removals for easier review:

diffstat refs/heads/master refs/heads/hton
 M  lib/libc/arch/aarch64/gen/byte_swap_2.S  |  0+  6-
 M  lib/libc/arch/aarch64/gen/byte_swap_4.S  |  0+  6-
 M  lib/libc/arch/arm/gen/byte_swap_2.S      |  0+  6-
 M  lib/libc/arch/arm/gen/byte_swap_4.S      |  0+  6-
 M  lib/libc/net/Makefile.inc                |  4+  9-
 M  lib/libc/net/htonl.c                     |  3+  9-
 M  lib/libc/net/htons.c                     |  3+  9-
 M  lib/libc/net/ntohl.c                     |  3+  9-
 M  lib/libc/net/ntohs.c                     |  3+  9-

9 files changed, 16 insertions(+), 69 deletions(-)

diff refs/heads/master refs/heads/hton
commit - 4733ced31b52279ffbbb7a95a03079ab09f79d05
commit + 51e061112e6521bdb8b75a45d7de812e88e1aa7c
blob - 306b40c0e3b6b9729b50ef7fdecafb1cf3d8a66f
blob + 1ca15676a229e04e44fb897a91df43e45f6f76a5
--- lib/libc/arch/aarch64/gen/byte_swap_2.S
+++ lib/libc/arch/aarch64/gen/byte_swap_2.S
@@ -33,16 +33,10 @@
 #include "DEFS.h"
 
 _ENTRY(__bswap16)
-_ENTRY_NB(ntohs)
-ENTRY_NB(htons)
 	RETGUARD_SETUP(__bswap16, x15)
 	and     w8, w0, #0xffff
         ubfx    w0, w0, #8, #8
 	bfi     w0, w8, #8, #16
 	RETGUARD_CHECK(__bswap16, x15)
 	ret
-END(htons)
-_END(ntohs)
 _END(__bswap16)
-	.weak	htons
-	.weak	ntohs
blob - 4d285b87c8676d24194fbd07b8d2872d64cbaf19
blob + 3b555baaee5eb111abb4125d94b2d26000b0c1a8
--- lib/libc/arch/aarch64/gen/byte_swap_4.S
+++ lib/libc/arch/aarch64/gen/byte_swap_4.S
@@ -33,14 +33,8 @@
 #include "DEFS.h"
 
 _ENTRY(__bswap32)
-_ENTRY_NB(ntohl)
-ENTRY_NB(htonl)
 	RETGUARD_SETUP(__bswap32, x15)
 	rev	w0, w0
 	RETGUARD_CHECK(__bswap32, x15)
 	ret
-END(htonl)
-_END(ntohl)
 _END(__bswap32)
-	.weak	htonl
-	.weak	ntohl
blob - 3429741ed9eb11d76b61956593f173c1a3ef3677
blob + ac4e78b14b543f904c18a2a738c23f31ec7e7507
--- lib/libc/arch/arm/gen/byte_swap_2.S
+++ lib/libc/arch/arm/gen/byte_swap_2.S
@@ -33,14 +33,8 @@
 #include "DEFS.h"
 
 _ENTRY(__bswap16)
-_ENTRY_NB(ntohs)
-ENTRY_NB(htons)
 	and		r1, r0, #0xff
 	mov		r0, r0, lsr #8
 	orr		r0, r0, r1, lsl #8
 	mov		pc, lr  
-END(htons)
-_END(ntohs)
 _END(__bswap16)
-	.weak	htons
-	.weak	ntohs
blob - 408e95448ae66fe23767b3e25c94249877acbe2d
blob + 82f037452a4f3f52ce94801d4aab208039257492
--- lib/libc/arch/arm/gen/byte_swap_4.S
+++ lib/libc/arch/arm/gen/byte_swap_4.S
@@ -33,15 +33,9 @@
 #include "DEFS.h"
 
 _ENTRY(__bswap32)
-_ENTRY_NB(ntohl)
-ENTRY_NB(htonl)
 	eor		r1, r0, r0, ror #16
 	bic		r1, r1, #0x00FF0000
 	mov		r0, r0, ror #8
 	eor		r0, r0, r1, lsr #8
 	mov		pc, lr
-END(htonl)
-_END(ntohl)
 _END(__bswap32)
-	.weak	htonl
-	.weak	ntohl
blob - 41a3164d6a95af2b5816be01f85df43776f5a5ff
blob + b8ff0fb931a32a54c00fbade18b0e6d551047c23
--- lib/libc/net/Makefile.inc
+++ lib/libc/net/Makefile.inc
@@ -1,7 +1,7 @@
 #	$OpenBSD: Makefile.inc,v 1.60 2019/08/30 18:33:17 deraadt Exp $
 
 # net sources
-.PATH: ${LIBCSRCDIR}/arch/${MACHINE_CPU}/net ${LIBCSRCDIR}/net
+.PATH: ${LIBCSRCDIR}/net
 
 CFLAGS+=-DRESOLVSORT
 
@@ -10,10 +10,11 @@ SRCS+=	base64.c ethers.c freeaddrinfo.c \
 	getifaddrs.c getnameinfo.c getnetent.c \
 	getnetnamadr.c getpeereid.c getproto.c getprotoent.c getprotoname.c \
 	getservbyname.c getservbyport.c getservent.c getrrsetbyname.c \
-	herror.c if_indextoname.c if_nameindex.c if_nametoindex.c inet_addr.c \
+	herror.c htonl.c htons.c \
+	if_indextoname.c if_nameindex.c if_nametoindex.c inet_addr.c \
 	inet_lnaof.c inet_makeaddr.c inet_neta.c inet_netof.c inet_network.c \
 	inet_net_ntop.c inet_net_pton.c inet_ntoa.c inet_ntop.c inet_pton.c \
-	linkaddr.c rcmd.c rcmdsh.c ruserok.c \
+	linkaddr.c ntohl.c ntohs.c rcmd.c rcmdsh.c ruserok.c \
 	rresvport.c recv.c res_comp.c res_data.c \
 	res_debug.c res_debug_syms.c res_init.c res_mkquery.c res_query.c \
 	res_random.c res_send.c \
@@ -22,12 +23,6 @@ SRCS+=	base64.c ethers.c freeaddrinfo.c \
 # IPv6
 SRCS+=	ip6opt.c rthdr.c vars6.c
 
-# machine-dependent net sources
-# m-d Makefile.inc must include sources for:
-#	htonl() htons() ntohl() ntohs()
-
-.include "${LIBCSRCDIR}/arch/${MACHINE_CPU}/net/Makefile.inc"
-
 MAN+=	htobe64.3 ether_aton.3 gai_strerror.3 getaddrinfo.3 gethostbyname.3 \
 	getifaddrs.3 getnameinfo.3 getnetent.3 getpeereid.3 getprotoent.3 \
 	getrrsetbyname.3 getservent.3 htonl.3 if_indextoname.3 \
blob - 6ee6e7efbf38908a62438ae4860e913c8e38ae7d
blob + 54f0ac057039a31d849cf6ef191836655a970bec
--- lib/libc/net/htonl.c
+++ lib/libc/net/htonl.c
@@ -1,6 +1,5 @@
 /*	$OpenBSD: htonl.c,v 1.7 2014/07/21 01:51:10 guenther Exp $ */
 /*
- * Written by J.T. Conklin <jtc@netbsd.org>.
  * Public domain.
  */
 
@@ -9,13 +8,8 @@
 
 #undef htonl
 
-u_int32_t
-htonl(u_int32_t x)
+uint32_t
+htonl(uint32_t x)
 {
-#if BYTE_ORDER == LITTLE_ENDIAN
-	u_char *s = (u_char *)&x;
-	return (u_int32_t)(s[0] << 24 | s[1] << 16 | s[2] << 8 | s[3]);
-#else
-	return x;
-#endif
+	return htobe32(x);
 }
blob - f48d91ee0358d09486f356c927b2df7217652ab2
blob + 5576f1e7f790779ade8a841b83aba1447d66479d
--- lib/libc/net/htons.c
+++ lib/libc/net/htons.c
@@ -1,6 +1,5 @@
 /*	$OpenBSD: htons.c,v 1.9 2014/07/21 01:51:10 guenther Exp $ */
 /*
- * Written by J.T. Conklin <jtc@netbsd.org>.
  * Public domain.
  */
 
@@ -9,13 +8,8 @@
 
 #undef htons
 
-u_int16_t
-htons(u_int16_t x)
+uint16_t
+htons(uint16_t x)
 {
-#if BYTE_ORDER == LITTLE_ENDIAN
-	u_char *s = (u_char *) &x;
-	return (u_int16_t)(s[0] << 8 | s[1]);
-#else
-	return x;
-#endif
+	return htobe16(x);
 }
blob - 0d05bac78a1c9ad61f7a7418022914c6704adfb4
blob + 23a66fedde7ac06d6549412e5bf03eb0c6f8bf7c
--- lib/libc/net/ntohl.c
+++ lib/libc/net/ntohl.c
@@ -1,6 +1,5 @@
 /*	$OpenBSD: ntohl.c,v 1.7 2014/07/21 01:51:10 guenther Exp $ */
 /*
- * Written by J.T. Conklin <jtc@netbsd.org>.
  * Public domain.
  */
 
@@ -9,13 +8,8 @@
 
 #undef ntohl
 
-u_int32_t
-ntohl(u_int32_t x)
+uint32_t
+ntohl(uint32_t x)
 {
-#if BYTE_ORDER == LITTLE_ENDIAN
-	u_char *s = (u_char *)&x;
-	return (u_int32_t)(s[0] << 24 | s[1] << 16 | s[2] << 8 | s[3]);
-#else
-	return x;
-#endif
+	return be32toh(x);
 }
blob - b5ea361f8304c322f46bc564e0e74a9a135d69c4
blob + 87fe25f334ada8449e1f2395c358bea33fe14d00
--- lib/libc/net/ntohs.c
+++ lib/libc/net/ntohs.c
@@ -1,6 +1,5 @@
 /*	$OpenBSD: ntohs.c,v 1.9 2014/07/21 01:51:10 guenther Exp $ */
 /*
- * Written by J.T. Conklin <jtc@netbsd.org>.
  * Public domain.
  */
 
@@ -9,13 +8,8 @@
 
 #undef ntohs
 
-u_int16_t
-ntohs(u_int16_t x)
+uint16_t
+ntohs(uint16_t x)
 {
-#if BYTE_ORDER == LITTLE_ENDIAN
-	u_char *s = (u_char *) &x;
-	return (u_int16_t)(s[0] << 8 | s[1]);
-#else
-	return x;
-#endif
+	return be16toh(x);
 }
-- 
Christian "naddy" Weisgerber                          naddy@mips.inka.de