| Message ID | 1314247456.27179.122.camel@deadeye |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Ben Hutchings <ben@decadent.org.uk> Date: Thu, 25 Aug 2011 05:44:15 +0100 > Commit 9c501935a3cdcf6b1d35aaee3aa11c7a7051a305 ('net: Support > inclusion of <linux/socket.h> before <sys/socket.h>') removed the > definition of struct sockaddr for userland. > > But we still have several headers using struct sockaddr, and we > shouldn't make them include <sys/socket.h> as that risks recursive > inclusion in future. Define and use an identical struct > __kernel_sockaddr instead. > > Signed-off-by: Ben Hutchings <ben@decadent.org.uk> ... > > -/* > - * 1003.1g requires sa_family_t and that sa_data is char. > - */ > +#define sockaddr __kernel_sockaddr This cure is worse than the disease. Now any application that includes sys/socket.h one way or another is now going to get a real hard build failure since __kernel_sockaddr will effectively get redefined. There is no reason sys/socket.h and our headers cannot co-exist and this is the whole reason we're going through all of this pain to use the __kernel_foo types in our networking datastructures which are exposed to userspace. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 26, 2011 at 11:08:20AM -0400, David Miller wrote: > From: Ben Hutchings <ben@decadent.org.uk> > Date: Thu, 25 Aug 2011 05:44:15 +0100 > > > Commit 9c501935a3cdcf6b1d35aaee3aa11c7a7051a305 ('net: Support > > inclusion of <linux/socket.h> before <sys/socket.h>') removed the > > definition of struct sockaddr for userland. > > > > But we still have several headers using struct sockaddr, and we > > shouldn't make them include <sys/socket.h> as that risks recursive > > inclusion in future. Define and use an identical struct > > __kernel_sockaddr instead. > > > > Signed-off-by: Ben Hutchings <ben@decadent.org.uk> > ... > > > > -/* > > - * 1003.1g requires sa_family_t and that sa_data is char. > > - */ > > +#define sockaddr __kernel_sockaddr > > This cure is worse than the disease. > > Now any application that includes sys/socket.h one way or another is > now going to get a real hard build failure since __kernel_sockaddr > will effectively get redefined. [...] This #define is in the #ifdef __KERNEL__ section. I don't want t change all the in-kernel users to say __kernel_sockaddr. This is just the same as the preceding 'typedef __kernel_sa_family_t sa_family_t' except that you can't create struct aliases that way. Ben.
From: Ben Hutchings <ben@decadent.org.uk> Date: Fri, 26 Aug 2011 16:42:48 +0100 > This #define is in the #ifdef __KERNEL__ section. I don't want t > change all the in-kernel users to say __kernel_sockaddr. This is > just the same as the preceding 'typedef __kernel_sa_family_t > sa_family_t' except that you can't create struct aliases that way. Aha, I didn't catch that part. Then this is fine, thanks for explaining Ben. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2011-08-26 at 12:20 -0400, David Miller wrote: > From: David Miller <davem@davemloft.net> > Date: Fri, 26 Aug 2011 12:04:51 -0400 (EDT) > > > From: Ben Hutchings <ben@decadent.org.uk> > > Date: Fri, 26 Aug 2011 16:42:48 +0100 > > > >> This #define is in the #ifdef __KERNEL__ section. I don't want t > >> change all the in-kernel users to say __kernel_sockaddr. This is > >> just the same as the preceding 'typedef __kernel_sa_family_t > >> sa_family_t' except that you can't create struct aliases that way. > > > > Aha, I didn't catch that part. Then this is fine, thanks for > > explaining Ben. > > Actually, this patch breaks the build, specifically of ifenslave.c > > I'll leave the other patches in, but you'll need to respin this one. [...] Right, it looks like this will have to wait for a proper resolution of the conflicts between kernel and libc types. Ben.
diff --git a/include/linux/if.h b/include/linux/if.h index db20bd4..f9ce5fb 100644 --- a/include/linux/if.h +++ b/include/linux/if.h @@ -20,7 +20,7 @@ #define _LINUX_IF_H #include <linux/types.h> /* for "__kernel_caddr_t" et al */ -#include <linux/socket.h> /* for "struct sockaddr" et al */ +#include <linux/socket.h> /* for "struct __kernel_sockaddr" et al */ #include <linux/compiler.h> /* for "__user" et al */ #define IFNAMSIZ 16 @@ -176,11 +176,11 @@ struct ifreq { } ifr_ifrn; union { - struct sockaddr ifru_addr; - struct sockaddr ifru_dstaddr; - struct sockaddr ifru_broadaddr; - struct sockaddr ifru_netmask; - struct sockaddr ifru_hwaddr; + struct __kernel_sockaddr ifru_addr; + struct __kernel_sockaddr ifru_dstaddr; + struct __kernel_sockaddr ifru_broadaddr; + struct __kernel_sockaddr ifru_netmask; + struct __kernel_sockaddr ifru_hwaddr; short ifru_flags; int ifru_ivalue; int ifru_mtu; diff --git a/include/linux/phonet.h b/include/linux/phonet.h index f53a416..9aba9ce 100644 --- a/include/linux/phonet.h +++ b/include/linux/phonet.h @@ -101,7 +101,8 @@ struct sockaddr_pn { __u8 spn_obj; __u8 spn_dev; __u8 spn_resource; - __u8 spn_zero[sizeof(struct sockaddr) - sizeof(__kernel_sa_family_t) - 3]; + __u8 spn_zero[sizeof(struct __kernel_sockaddr) - + sizeof(__kernel_sa_family_t) - 3]; } __attribute__((packed)); /* Well known address */ diff --git a/include/linux/socket.h b/include/linux/socket.h index d0e77f6..f13ad7b 100644 --- a/include/linux/socket.h +++ b/include/linux/socket.h @@ -10,6 +10,14 @@ typedef unsigned short __kernel_sa_family_t; +/* + * 1003.1g requires sa_family_t and that sa_data is char. + */ +struct __kernel_sockaddr { + __kernel_sa_family_t sa_family; /* address family, AF_xxx */ + char sa_data[14]; /* 14 bytes of protocol address */ +}; + struct __kernel_sockaddr_storage { __kernel_sa_family_t ss_family; /* address family */ /* Following field(s) are implementation specific */ @@ -39,15 +47,8 @@ extern void socket_seq_show(struct seq_file *seq); typedef __kernel_sa_family_t sa_family_t; -/* - * 1003.1g requires sa_family_t and that sa_data is char. - */ +#define sockaddr __kernel_sockaddr -struct sockaddr { - sa_family_t sa_family; /* address family, AF_xxx */ - char sa_data[14]; /* 14 bytes of protocol address */ -}; - struct linger { int l_onoff; /* Linger active */ int l_linger; /* How long to linger for */
Commit 9c501935a3cdcf6b1d35aaee3aa11c7a7051a305 ('net: Support inclusion of <linux/socket.h> before <sys/socket.h>') removed the definition of struct sockaddr for userland. But we still have several headers using struct sockaddr, and we shouldn't make them include <sys/socket.h> as that risks recursive inclusion in future. Define and use an identical struct __kernel_sockaddr instead. Signed-off-by: Ben Hutchings <ben@decadent.org.uk> --- include/linux/if.h | 12 ++++++------ include/linux/phonet.h | 3 ++- include/linux/socket.h | 17 +++++++++-------- 3 files changed, 17 insertions(+), 15 deletions(-)