Message ID | 20160830011645.25769-4-zackw@panix.com |
---|---|
State | New |
Headers | show |
On Mon, 29 Aug 2016, Zack Weinberg wrote: > These are all fallout from the change from u_intNN_t to uintNN_t; a > number of headers now need to include <stdint.h> to pick up those > types. It is possible that __uintNN_t should be used instead. I'm not clear whether this is needed to restore the glibc build / testsuite build, or only for installed use. If it's needed to keep things building, it has to come *before* the patch that causes the header to be required, to keep bisectability.
On Mon, Aug 29, 2016 at 9:23 PM, Joseph Myers <joseph@codesourcery.com> wrote: > On Mon, 29 Aug 2016, Zack Weinberg wrote: > >> These are all fallout from the change from u_intNN_t to uintNN_t; a >> number of headers now need to include <stdint.h> to pick up those >> types. It is possible that __uintNN_t should be used instead. > > I'm not clear whether this is needed to restore the glibc build / > testsuite build, or only for installed use. If it's needed to keep things > building, it has to come *before* the patch that causes the header to be > required, to keep bisectability. It is necessary to restore the build. This is only separate for ease of review; my plan was always to squash the entire series into one big patch for landing. Or maybe two or three big patches - the time.h and ucontext.h changes qualify as separate ideas, probably. zw
On 08/29/2016 09:16 PM, Zack Weinberg wrote: > These are all fallout from the change from u_intNN_t to uintNN_t; a > number of headers now need to include <stdint.h> to pick up those > types. It is possible that __uintNN_t should be used instead. Why should __uintNN_t ever be used? At a minimum the uintNN_t types should be available regardless of whatever mode you are being compiled in? > Some of these files directly included <features.h> and/or > <sys/cdefs.h>, which I removed, as the style generally seems to be to > let <sys/types.h> do that for us. One file included <asm/types.h> > gratuitously. Are you certain that was safe? I would be worried about arbitrarily removing those headers without a deeper analysis. > * inet/protocols/talkd.h, resolv/arpa/nameser.h > * sysdeps/generic/netinet/in_systm.h > * sysdeps/gnu/netinet/ip_icmp.h, sysdeps/gnu/netinet/tcp.h > * sysdeps/gnu/netinet/udp.h > * sysdeps/unix/sysv/linux/net/ethernet.h > * sysdeps/unix/sysv/linux/net/if_arp.h > * sysdeps/unix/sysv/linux/net/if_ppp.h > * sysdeps/unix/sysv/linux/net/if_shaper.h > * sysdeps/unix/sysv/linux/netinet/if_fddi.h > * sysdeps/unix/sysv/linux/netinet/if_tr.h > * sysdeps/unix/sysv/linux/netipx/ipx.h > * sysdeps/unix/sysv/linux/sys/acct.h > * sysdeps/unix/sysv/linux/sys/quota.h: > Include stdint.h for uintNN_t definitions. > Don't include sys/cdefs.h, features.h, or asm/types.h directly. OK only if you verified that the generated code did not change due to the removal of unneeded headers, or if you drop the unneeded header removal.
On 08/29/2016 11:11 PM, Zack Weinberg wrote: > On Mon, Aug 29, 2016 at 9:23 PM, Joseph Myers <joseph@codesourcery.com> wrote: >> On Mon, 29 Aug 2016, Zack Weinberg wrote: >> >>> These are all fallout from the change from u_intNN_t to uintNN_t; a >>> number of headers now need to include <stdint.h> to pick up those >>> types. It is possible that __uintNN_t should be used instead. >> >> I'm not clear whether this is needed to restore the glibc build / >> testsuite build, or only for installed use. If it's needed to keep things >> building, it has to come *before* the patch that causes the header to be >> required, to keep bisectability. > > It is necessary to restore the build. This is only separate for ease > of review; my plan was always to squash the entire series into one big > patch for landing. Or maybe two or three big patches - the time.h and > ucontext.h changes qualify as separate ideas, probably. That was my understanding. That this is going to be a single patch that does the mechanical changes in one commit which can be reverted, as suggested in patch 00/13.
On Wed, 21 Sep 2016, Carlos O'Donell wrote: > On 08/29/2016 09:16 PM, Zack Weinberg wrote: > > These are all fallout from the change from u_intNN_t to uintNN_t; a > > number of headers now need to include <stdint.h> to pick up those > > types. It is possible that __uintNN_t should be used instead. > > Why should __uintNN_t ever be used? At a minimum the uintNN_t types > should be available regardless of whatever mode you are being compiled in? ISO C doesn't reserve *_t globally, but unless any of these headers are ISO C headers that's not a relevant consideration (and if inappropriate *_t types were being added to ISO C headers that would have been detected by the conform/ tests).
On Wed, Sep 21, 2016 at 1:52 PM, Carlos O'Donell <carlos@redhat.com> wrote: > On 08/29/2016 09:16 PM, Zack Weinberg wrote: >> These are all fallout from the change from u_intNN_t to uintNN_t; a >> number of headers now need to include <stdint.h> to pick up those >> types. It is possible that __uintNN_t should be used instead. > > Why should __uintNN_t ever be used? At a minimum the uintNN_t types > should be available regardless of whatever mode you are being compiled in? It might be inappropriate for some of these headers to (behave as-if they) include <stdint.h>. Joseph says that existing conform/ tests cover this from a standards-compliance perspective, but it is my understanding that net/* and netinet/* headers are not very well standardized, so we might want to think about whether we should go to extra effort here. >> Some of these files directly included <features.h> and/or >> <sys/cdefs.h>, which I removed, as the style generally seems to be to >> let <sys/types.h> do that for us. One file included <asm/types.h> >> gratuitously. > > Are you certain that was safe? I would be worried about arbitrarily > removing those headers without a deeper analysis. sys/types.h unconditionally includes features.h, and features.h unconditionally includes sys/cdefs.h. None of the definitions in asm/types.h were required by the file that was including it. I will do a check for unchanged installed libraries as discussed elsewhere. zw
On 09/21/2016 02:22 PM, Zack Weinberg wrote: > On Wed, Sep 21, 2016 at 1:52 PM, Carlos O'Donell <carlos@redhat.com> wrote: >> On 08/29/2016 09:16 PM, Zack Weinberg wrote: >>> These are all fallout from the change from u_intNN_t to uintNN_t; a >>> number of headers now need to include <stdint.h> to pick up those >>> types. It is possible that __uintNN_t should be used instead. >> >> Why should __uintNN_t ever be used? At a minimum the uintNN_t types >> should be available regardless of whatever mode you are being compiled in? > > It might be inappropriate for some of these headers to (behave as-if > they) include <stdint.h>. Joseph says that existing conform/ tests > cover this from a standards-compliance perspective, but it is my > understanding that net/* and netinet/* headers are not very well > standardized, so we might want to think about whether we should go to > extra effort here. Thanks for the clarification. Given the underspecified nature of those headers I don't see that we need to worry. If someone comes back and has a valid complaint we could always do a further cleanup? >>> Some of these files directly included <features.h> and/or >>> <sys/cdefs.h>, which I removed, as the style generally seems to be to >>> let <sys/types.h> do that for us. One file included <asm/types.h> >>> gratuitously. >> >> Are you certain that was safe? I would be worried about arbitrarily >> removing those headers without a deeper analysis. > > sys/types.h unconditionally includes features.h, and features.h > unconditionally includes sys/cdefs.h. > > None of the definitions in asm/types.h were required by the file that > was including it. > > I will do a check for unchanged installed libraries as discussed elsewhere. Belt-and-suspenders ;-)
diff --git a/inet/protocols/talkd.h b/inet/protocols/talkd.h index 0437ae4..34e2654 100644 --- a/inet/protocols/talkd.h +++ b/inet/protocols/talkd.h @@ -52,6 +52,7 @@ #include <sys/types.h> #include <sys/socket.h> +#include <stdint.h> /* * Client->server request message format. diff --git a/resolv/arpa/nameser.h b/resolv/arpa/nameser.h index 47240c7..62c11e0 100644 --- a/resolv/arpa/nameser.h +++ b/resolv/arpa/nameser.h @@ -58,7 +58,7 @@ #include <sys/param.h> #include <sys/types.h> -#include <sys/cdefs.h> +#include <stdint.h> /*% * Revision information. This is the release date in YYYYMMDD format. diff --git a/sysdeps/generic/netinet/in_systm.h b/sysdeps/generic/netinet/in_systm.h index 1629c36..7b9a92b 100644 --- a/sysdeps/generic/netinet/in_systm.h +++ b/sysdeps/generic/netinet/in_systm.h @@ -19,8 +19,8 @@ #ifndef _NETINET_IN_SYSTM_H #define _NETINET_IN_SYSTM_H 1 -#include <sys/cdefs.h> #include <sys/types.h> +#include <stdint.h> __BEGIN_DECLS diff --git a/sysdeps/gnu/netinet/ip_icmp.h b/sysdeps/gnu/netinet/ip_icmp.h index e57b144..542e789 100644 --- a/sysdeps/gnu/netinet/ip_icmp.h +++ b/sysdeps/gnu/netinet/ip_icmp.h @@ -18,8 +18,8 @@ #ifndef __NETINET_IP_ICMP_H #define __NETINET_IP_ICMP_H 1 -#include <sys/cdefs.h> #include <sys/types.h> +#include <stdint.h> __BEGIN_DECLS diff --git a/sysdeps/gnu/netinet/tcp.h b/sysdeps/gnu/netinet/tcp.h index 42ec108..3fbea54 100644 --- a/sysdeps/gnu/netinet/tcp.h +++ b/sysdeps/gnu/netinet/tcp.h @@ -73,6 +73,7 @@ #ifdef __USE_MISC # include <sys/types.h> # include <sys/socket.h> +# include <stdint.h> typedef uint32_t tcp_seq; /* diff --git a/sysdeps/gnu/netinet/udp.h b/sysdeps/gnu/netinet/udp.h index f55391c..d5f60e4 100644 --- a/sysdeps/gnu/netinet/udp.h +++ b/sysdeps/gnu/netinet/udp.h @@ -47,9 +47,8 @@ #ifndef __NETINET_UDP_H #define __NETINET_UDP_H 1 -#include <features.h> #include <sys/types.h> - +#include <stdint.h> /* UDP header as specified by RFC 768, August 1980. */ diff --git a/sysdeps/unix/sysv/linux/net/ethernet.h b/sysdeps/unix/sysv/linux/net/ethernet.h index 56b3276..833473e 100644 --- a/sysdeps/unix/sysv/linux/net/ethernet.h +++ b/sysdeps/unix/sysv/linux/net/ethernet.h @@ -21,8 +21,9 @@ #ifndef __NET_ETHERNET_H #define __NET_ETHERNET_H 1 -#include <sys/cdefs.h> #include <sys/types.h> +#include <stdint.h> + #include <linux/if_ether.h> /* IEEE 802.3 Ethernet constants */ __BEGIN_DECLS diff --git a/sysdeps/unix/sysv/linux/net/if_arp.h b/sysdeps/unix/sysv/linux/net/if_arp.h index 93758c8..9a20c83 100644 --- a/sysdeps/unix/sysv/linux/net/if_arp.h +++ b/sysdeps/unix/sysv/linux/net/if_arp.h @@ -20,12 +20,11 @@ /* Based on the 4.4BSD and Linux version of this file. */ #ifndef _NET_IF_ARP_H - #define _NET_IF_ARP_H 1 -#include <sys/cdefs.h> #include <sys/types.h> #include <sys/socket.h> +#include <stdint.h> __BEGIN_DECLS diff --git a/sysdeps/unix/sysv/linux/net/if_ppp.h b/sysdeps/unix/sysv/linux/net/if_ppp.h index 20310eb..9994982 100644 --- a/sysdeps/unix/sysv/linux/net/if_ppp.h +++ b/sysdeps/unix/sysv/linux/net/if_ppp.h @@ -49,8 +49,7 @@ #define __NET_IF_PPP_H 1 #include <sys/types.h> -#include <sys/cdefs.h> - +#include <stdint.h> #include <net/if.h> #include <sys/ioctl.h> #include <net/ppp_defs.h> diff --git a/sysdeps/unix/sysv/linux/net/if_shaper.h b/sysdeps/unix/sysv/linux/net/if_shaper.h index 4a777d8..e318794 100644 --- a/sysdeps/unix/sysv/linux/net/if_shaper.h +++ b/sysdeps/unix/sysv/linux/net/if_shaper.h @@ -18,8 +18,8 @@ #ifndef _NET_IF_SHAPER_H #define _NET_IF_SHAPER_H 1 -#include <features.h> #include <sys/types.h> +#include <stdint.h> #include <net/if.h> #include <sys/ioctl.h> diff --git a/sysdeps/unix/sysv/linux/netinet/if_fddi.h b/sysdeps/unix/sysv/linux/netinet/if_fddi.h index 877f738..6758014 100644 --- a/sysdeps/unix/sysv/linux/netinet/if_fddi.h +++ b/sysdeps/unix/sysv/linux/netinet/if_fddi.h @@ -18,10 +18,8 @@ #ifndef _NETINET_IF_FDDI_H #define _NETINET_IF_FDDI_H 1 -#include <sys/cdefs.h> #include <sys/types.h> -#include <asm/types.h> - +#include <stdint.h> #include <linux/if_fddi.h> #ifdef __USE_MISC diff --git a/sysdeps/unix/sysv/linux/netinet/if_tr.h b/sysdeps/unix/sysv/linux/netinet/if_tr.h index 5df2206..814a35d 100644 --- a/sysdeps/unix/sysv/linux/netinet/if_tr.h +++ b/sysdeps/unix/sysv/linux/netinet/if_tr.h @@ -18,8 +18,8 @@ #ifndef _NETINET_IF_TR_H #define _NETINET_IF_TR_H 1 -#include <sys/cdefs.h> #include <sys/types.h> +#include <stdint.h> /* IEEE 802.5 Token-Ring magic constants. The frame sizes omit the preamble and FCS/CRC (frame check sequence). */ diff --git a/sysdeps/unix/sysv/linux/netipx/ipx.h b/sysdeps/unix/sysv/linux/netipx/ipx.h index 1d6cb78..338aab5 100644 --- a/sysdeps/unix/sysv/linux/netipx/ipx.h +++ b/sysdeps/unix/sysv/linux/netipx/ipx.h @@ -18,9 +18,8 @@ #ifndef __NETIPX_IPX_H #define __NETIPX_IPX_H 1 -#include <features.h> - #include <sys/types.h> +#include <stdint.h> #include <bits/sockaddr.h> __BEGIN_DECLS diff --git a/sysdeps/unix/sysv/linux/sys/acct.h b/sysdeps/unix/sysv/linux/sys/acct.h index c629ad4..d24c2a7 100644 --- a/sysdeps/unix/sysv/linux/sys/acct.h +++ b/sysdeps/unix/sysv/linux/sys/acct.h @@ -18,12 +18,11 @@ #ifndef _SYS_ACCT_H #define _SYS_ACCT_H 1 -#include <features.h> - +#include <sys/types.h> +#include <stdint.h> #include <endian.h> #define __need_time_t #include <time.h> -#include <sys/types.h> __BEGIN_DECLS diff --git a/sysdeps/unix/sysv/linux/sys/quota.h b/sysdeps/unix/sysv/linux/sys/quota.h index d675664..3e6b4ba 100644 --- a/sysdeps/unix/sysv/linux/sys/quota.h +++ b/sysdeps/unix/sysv/linux/sys/quota.h @@ -35,8 +35,8 @@ #ifndef _SYS_QUOTA_H #define _SYS_QUOTA_H 1 -#include <features.h> #include <sys/types.h> +#include <stdint.h> /* * Select between different incompatible quota versions.