Message ID | 1469919569-27798-1-git-send-email-aurelien@aurel32.net |
---|---|
State | New |
Headers | show |
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
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: > +# 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).
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
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)