* [PATCH] Fix htonl, htons, ntohl, ntohs functions on big endian systems
@ 2016-07-31 1:40 Aurelien Jarno
2016-07-31 8:59 ` Zack Weinberg
2016-07-31 18:45 ` Florian Weimer
0 siblings, 2 replies; 6+ messages in thread
From: Aurelien Jarno @ 2016-07-31 1:40 UTC (permalink / raw)
To: libc-alpha; +Cc: Aurelien Jarno
The htonl, htons, ntohl and ntohs funtions are defined by POSIX as:
uint32_t htonl(uint32_t hostlong);
uint16_t htons(uint16_t hostshort);
uint32_t ntohl(uint32_t netlong);
uint16_t ntohs(uint16_t netshort);
They take a uint16_t or uint32_t argument and return a value with the
same type. This means for example that calling htonl on a 64-bit value
should return a 32-bit value.
The GNU libc implements these functions as macros when optimizations are
enabled, and this is explicitely allowed by POSIX. However on big endian
systems there are then defined as:
# define ntohl(x) (x)
# define ntohs(x) (x)
# define htonl(x) (x)
# define htons(x) (x)
This means that the values are not casted if the argument is bigger. In
turns that means that:
- the behaviour is different between little and big endian systems;
- the behaviour depends on the optimization level.
This patch attempts to fix that. It adds an implicit cast for ntohl and
htonl using a GCC extension or an explicit one when not using GCC. It
adds an explicit cast for ntohs and htons as the call to __bswap_16
contains an explicit cast.
Changelog:
* inet/netinet/in.h [__OPTIMIZE__ && (__BYTE_ORDER == __BIG_ENDIAN)]
(ntohl): Add an implicit uint32_t cast if [__GNUC__ >= 2] or an
explicit uint32_t cast if [!__GNUC__ >= 2].
[__OPTIMIZE__ && (__BYTE_ORDER == __BIG_ENDIAN)] (htonl): Likewise.
[__OPTIMIZE__ && (__BYTE_ORDER == __BIG_ENDIAN)] (ntohs): Add a uint16_t
cast.
[__OPTIMIZE__ && (__BYTE_ORDER == __BIG_ENDIAN)] (htons): Likewise.
---
ChangeLog | 10 ++++++++++
inet/netinet/in.h | 19 +++++++++++++++----
2 files changed, 25 insertions(+), 4 deletions(-)
Note: the macros in <endian.h> also suffer from the same issue. I'll
send a patch for them if this change is considered acceptable.
diff --git a/ChangeLog b/ChangeLog
index f148ac8..ee1b091 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2016-07-31 Aurelien Jarno <aurelien@aurel32.net>
+
+ * inet/netinet/in.h [__OPTIMIZE__ && (__BYTE_ORDER == __BIG_ENDIAN)]
+ (ntohl): Add an implicit uint32_t cast if [__GNUC__ >= 2] or an
+ explicit uint32_t cast if [!__GNUC__ >= 2].
+ [__OPTIMIZE__ && (__BYTE_ORDER == __BIG_ENDIAN)] (htonl): Likewise.
+ [__OPTIMIZE__ && (__BYTE_ORDER == __BIG_ENDIAN)] (ntohs): Add a uint16_t
+ cast.
+ [__OPTIMIZE__ && (__BYTE_ORDER == __BIG_ENDIAN)] (htons): Likewise.
+
2016-07-27 H.J. Lu <hongjiu.lu@intel.com>
[BZ #20384]
diff --git a/inet/netinet/in.h b/inet/netinet/in.h
index c801593..86e812a 100644
--- a/inet/netinet/in.h
+++ b/inet/netinet/in.h
@@ -393,10 +393,21 @@ extern uint16_t htons (uint16_t __hostshort)
# if __BYTE_ORDER == __BIG_ENDIAN
/* The host byte order is the same as network byte order,
so these functions are all just identity. */
-# define ntohl(x) (x)
-# define ntohs(x) (x)
-# define htonl(x) (x)
-# define htons(x) (x)
+# if defined __GNUC__ && __GNUC__ >= 2
+# define ntohl(x) \
+ (__extension__ \
+ ({ uint32_t __x = x; \
+ __x; }))
+# define htonl(x) \
+ (__extension__ \
+ ({ uint32_t __x = x; \
+ __x; }))
+# else
+# define ntohl(x) ((uint32_t) (x))
+# define htonl(x) ((uint32_t) (x))
+# endif
+# define ntohs(x) ((uint16_t) (x))
+# define htons(x) ((uint16_t) (x))
# else
# if __BYTE_ORDER == __LITTLE_ENDIAN
# define ntohl(x) __bswap_32 (x)
--
2.8.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix htonl, htons, ntohl, ntohs functions on big endian systems
2016-07-31 1:40 [PATCH] Fix htonl, htons, ntohl, ntohs functions on big endian systems Aurelien Jarno
@ 2016-07-31 8:59 ` Zack Weinberg
2016-07-31 9:17 ` Aurelien Jarno
2016-07-31 18:45 ` Florian Weimer
1 sibling, 1 reply; 6+ messages in thread
From: Zack Weinberg @ 2016-07-31 8:59 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: GNU C Library
On Sat, Jul 30, 2016 at 6:59 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
>
> This patch attempts to fix that. It adds an implicit cast for ntohl and
> htonl using a GCC extension or an explicit one when not using GCC. It
> adds an explicit cast for ntohs and htons as the call to __bswap_16
> contains an explicit cast.
I support fixing this. However, what is the benefit of the "implicit
cast" construct over the "explicit"? It seems like gratuitous
#ifdeffage at first sight.
zw
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix htonl, htons, ntohl, ntohs functions on big endian systems
2016-07-31 8:59 ` Zack Weinberg
@ 2016-07-31 9:17 ` Aurelien Jarno
2016-07-31 20:09 ` Zack Weinberg
0 siblings, 1 reply; 6+ messages in thread
From: Aurelien Jarno @ 2016-07-31 9:17 UTC (permalink / raw)
To: Zack Weinberg; +Cc: GNU C Library
On 2016-07-30 21:55, Zack Weinberg wrote:
> On Sat, Jul 30, 2016 at 6:59 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> >
> > This patch attempts to fix that. It adds an implicit cast for ntohl and
> > htonl using a GCC extension or an explicit one when not using GCC. It
> > adds an explicit cast for ntohs and htons as the call to __bswap_16
> > contains an explicit cast.
>
> I support fixing this. However, what is the benefit of the "implicit
> cast" construct over the "explicit"? It seems like gratuitous
> #ifdeffage at first sight.
With the implicit cast, GCC still emits a wining when using
-Wconversion, just like using on little endian.
Aurelien
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix htonl, htons, ntohl, ntohs functions on big endian systems
2016-07-31 1:40 [PATCH] Fix htonl, htons, ntohl, ntohs functions on big endian systems Aurelien Jarno
2016-07-31 8:59 ` Zack Weinberg
@ 2016-07-31 18:45 ` Florian Weimer
1 sibling, 0 replies; 6+ messages in thread
From: Florian Weimer @ 2016-07-31 18:45 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: libc-alpha
* Aurelien Jarno:
> +# if defined __GNUC__ && __GNUC__ >= 2
> +# define ntohl(x) \
> + (__extension__ \
> + ({ uint32_t __x = x; \
Please use __ntohl_x as the variable name or turn ntohl into a macro
which calls an inline function. ntohl (__x) looks like something
another header file might do (although the example in gutenprint is
harmless).
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix htonl, htons, ntohl, ntohs functions on big endian systems
2016-07-31 9:17 ` Aurelien Jarno
@ 2016-07-31 20:09 ` Zack Weinberg
2016-08-01 4:46 ` Mike Frysinger
0 siblings, 1 reply; 6+ messages in thread
From: Zack Weinberg @ 2016-07-31 20:09 UTC (permalink / raw)
To: Zack Weinberg, GNU C Library
On Sun, Jul 31, 2016 at 4:59 AM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On 2016-07-30 21:55, Zack Weinberg wrote:
>> On Sat, Jul 30, 2016 at 6:59 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
>> >
>> > This patch attempts to fix that. It adds an implicit cast for ntohl and
>> > htonl using a GCC extension or an explicit one when not using GCC. It
>> > adds an explicit cast for ntohs and htons as the call to __bswap_16
>> > contains an explicit cast.
>>
>> I support fixing this. However, what is the benefit of the "implicit
>> cast" construct over the "explicit"? It seems like gratuitous
>> #ifdeffage at first sight.
>
> With the implicit cast, GCC still emits a wining when using
> -Wconversion, just like using on little endian.
Oh, I see. In that case I think I vote for an inline function instead
(this also addresses Florian's concern).
zw
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix htonl, htons, ntohl, ntohs functions on big endian systems
2016-07-31 20:09 ` Zack Weinberg
@ 2016-08-01 4:46 ` Mike Frysinger
0 siblings, 0 replies; 6+ messages in thread
From: Mike Frysinger @ 2016-08-01 4:46 UTC (permalink / raw)
To: Zack Weinberg; +Cc: GNU C Library
[-- Attachment #1: Type: text/plain, Size: 979 bytes --]
On 31 Jul 2016 14:45, Zack Weinberg wrote:
> On Sun, Jul 31, 2016 at 4:59 AM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> > On 2016-07-30 21:55, Zack Weinberg wrote:
> >> On Sat, Jul 30, 2016 at 6:59 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> >> >
> >> > This patch attempts to fix that. It adds an implicit cast for ntohl and
> >> > htonl using a GCC extension or an explicit one when not using GCC. It
> >> > adds an explicit cast for ntohs and htons as the call to __bswap_16
> >> > contains an explicit cast.
> >>
> >> I support fixing this. However, what is the benefit of the "implicit
> >> cast" construct over the "explicit"? It seems like gratuitous
> >> #ifdeffage at first sight.
> >
> > With the implicit cast, GCC still emits a wining when using
> > -Wconversion, just like using on little endian.
>
> Oh, I see. In that case I think I vote for an inline function instead
> (this also addresses Florian's concern).
agreed
-mike
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-08-01 4:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-31 1:40 [PATCH] Fix htonl, htons, ntohl, ntohs functions on big endian systems Aurelien Jarno
2016-07-31 8:59 ` Zack Weinberg
2016-07-31 9:17 ` Aurelien Jarno
2016-07-31 20:09 ` Zack Weinberg
2016-08-01 4:46 ` Mike Frysinger
2016-07-31 18:45 ` Florian Weimer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).