| Message ID | 20200521201422.16493-1-hauke@hauke-m.de (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | wireless: Use linux/stddef.h instead of stddef.h | expand |
On Thursday 21 May 2020 22:14:22 CEST Hauke Mehrtens wrote: > When compiling inside the kernel include linux/stddef.h instead of > stddef.h. When I compile this header file in backports for power PC I > run into a conflict with ptrdiff_t. I was unable to reproduce this in > mainline kernel. I still would like to fix this problem in the kernel. > > Fixes: 6989310f5d43 ("wireless: Use offsetof instead of custom macro.") > Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> > --- > include/uapi/linux/wireless.h | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/wireless.h b/include/uapi/linux/wireless.h > index a2c006a364e0..24f3371ad826 100644 > --- a/include/uapi/linux/wireless.h > +++ b/include/uapi/linux/wireless.h > @@ -74,7 +74,11 @@ > #include <linux/socket.h> /* for "struct sockaddr" et al */ > #include <linux/if.h> /* for IFNAMSIZ and co... */ > > -#include <stddef.h> /* for offsetof */ > +#ifdef __KERNEL__ > +# include <linux/stddef.h> /* for offsetof */ > +#else > +# include <stddef.h> /* for offsetof */ > +#endif Hello, This patch also solves a warning I encountered when I launched sparse (with make C=1): /usr/lib/gcc-cross/arm-linux-gnueabihf/9/include/stddef.h:406:9: warning: preprocessor token offsetof redefined ./include/linux/stddef.h:17:9: this was the original definition However, I wonder if it useful to keep 'include <stddef.h>' when this file is included from userspace. If it does not compile anymore, there is problem somewhere else, no? (Globally, I always find suspicious a kernel source that includes a system header)
On 5/25/20 11:35 AM, Jérôme Pouiller wrote: > On Thursday 21 May 2020 22:14:22 CEST Hauke Mehrtens wrote: >> When compiling inside the kernel include linux/stddef.h instead of >> stddef.h. When I compile this header file in backports for power PC I >> run into a conflict with ptrdiff_t. I was unable to reproduce this in >> mainline kernel. I still would like to fix this problem in the kernel. >> >> Fixes: 6989310f5d43 ("wireless: Use offsetof instead of custom macro.") >> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> >> --- >> include/uapi/linux/wireless.h | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/include/uapi/linux/wireless.h b/include/uapi/linux/wireless.h >> index a2c006a364e0..24f3371ad826 100644 >> --- a/include/uapi/linux/wireless.h >> +++ b/include/uapi/linux/wireless.h >> @@ -74,7 +74,11 @@ >> #include <linux/socket.h> /* for "struct sockaddr" et al */ >> #include <linux/if.h> /* for IFNAMSIZ and co... */ >> >> -#include <stddef.h> /* for offsetof */ >> +#ifdef __KERNEL__ >> +# include <linux/stddef.h> /* for offsetof */ >> +#else >> +# include <stddef.h> /* for offsetof */ >> +#endif > > Hello, > > This patch also solves a warning I encountered when I launched sparse > (with make C=1): > > /usr/lib/gcc-cross/arm-linux-gnueabihf/9/include/stddef.h:406:9: warning: preprocessor token offsetof redefined > ./include/linux/stddef.h:17:9: this was the original definition > > However, I wonder if it useful to keep 'include <stddef.h>' when this file > is included from userspace. If it does not compile anymore, there is > problem somewhere else, no? > > (Globally, I always find suspicious a kernel source that includes a system > header) Hi, The wireless.h makes use of offsetof and this is defined in system stddef.h and in include/linux/stddef.h. When we only include linux/stddef.h it will work fine when compiled inside the kernel, but it could cause problems when used in user space, because this would include ./include/uapi/linux/stddef.h which does not define offsetof. I think we need the #ifdef __KERNEL__ to make this work in the kernel and in the user space. Hauke
On Tuesday 26 May 2020 23:02:56 CEST Hauke Mehrtens wrote: > On 5/25/20 11:35 AM, Jérôme Pouiller wrote: > > On Thursday 21 May 2020 22:14:22 CEST Hauke Mehrtens wrote: > >> When compiling inside the kernel include linux/stddef.h instead of > >> stddef.h. When I compile this header file in backports for power PC I > >> run into a conflict with ptrdiff_t. I was unable to reproduce this in > >> mainline kernel. I still would like to fix this problem in the kernel. > >> > >> Fixes: 6989310f5d43 ("wireless: Use offsetof instead of custom macro.") > >> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> > >> --- > >> include/uapi/linux/wireless.h | 6 +++++- > >> 1 file changed, 5 insertions(+), 1 deletion(-) > >> > >> diff --git a/include/uapi/linux/wireless.h b/include/uapi/linux/wireless.h > >> index a2c006a364e0..24f3371ad826 100644 > >> --- a/include/uapi/linux/wireless.h > >> +++ b/include/uapi/linux/wireless.h > >> @@ -74,7 +74,11 @@ > >> #include <linux/socket.h> /* for "struct sockaddr" et al */ > >> #include <linux/if.h> /* for IFNAMSIZ and co... */ > >> > >> -#include <stddef.h> /* for offsetof */ > >> +#ifdef __KERNEL__ > >> +# include <linux/stddef.h> /* for offsetof */ > >> +#else > >> +# include <stddef.h> /* for offsetof */ > >> +#endif > > > > Hello, > > > > This patch also solves a warning I encountered when I launched sparse > > (with make C=1): > > > > /usr/lib/gcc-cross/arm-linux-gnueabihf/9/include/stddef.h:406:9: warning: preprocessor token offsetof redefined > > ./include/linux/stddef.h:17:9: this was the original definition > > > > However, I wonder if it useful to keep 'include <stddef.h>' when this file > > is included from userspace. If it does not compile anymore, there is > > problem somewhere else, no? > > > > (Globally, I always find suspicious a kernel source that includes a system > > header) > Hi, > > The wireless.h makes use of offsetof and this is defined in system > stddef.h and in include/linux/stddef.h. When we only include > linux/stddef.h it will work fine when compiled inside the kernel, but it > could cause problems when used in user space, because this would include > ./include/uapi/linux/stddef.h which does not define offsetof. > > I think we need the #ifdef __KERNEL__ to make this work in the kernel > and in the user space. In this case, maybe the problem should be resolved directly in uapi/linux/stddef.h? I have found other headers that use offsetof and may have the same problem (currently, they do not include stddef.h. So, some of the macros do not compile): - include/uapi/linux/fuse.h - include/uapi/linux/genwqe/genwqe_card.h - include/uapi/linux/gfs2_ondisk.h
On 5/27/20 4:03 PM, Jérôme Pouiller wrote: > On Tuesday 26 May 2020 23:02:56 CEST Hauke Mehrtens wrote: >> On 5/25/20 11:35 AM, Jérôme Pouiller wrote: >>> On Thursday 21 May 2020 22:14:22 CEST Hauke Mehrtens wrote: >>>> When compiling inside the kernel include linux/stddef.h instead of >>>> stddef.h. When I compile this header file in backports for power PC I >>>> run into a conflict with ptrdiff_t. I was unable to reproduce this in >>>> mainline kernel. I still would like to fix this problem in the kernel. >>>> >>>> Fixes: 6989310f5d43 ("wireless: Use offsetof instead of custom macro.") >>>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> >>>> --- >>>> include/uapi/linux/wireless.h | 6 +++++- >>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/include/uapi/linux/wireless.h b/include/uapi/linux/wireless.h >>>> index a2c006a364e0..24f3371ad826 100644 >>>> --- a/include/uapi/linux/wireless.h >>>> +++ b/include/uapi/linux/wireless.h >>>> @@ -74,7 +74,11 @@ >>>> #include <linux/socket.h> /* for "struct sockaddr" et al */ >>>> #include <linux/if.h> /* for IFNAMSIZ and co... */ >>>> >>>> -#include <stddef.h> /* for offsetof */ >>>> +#ifdef __KERNEL__ >>>> +# include <linux/stddef.h> /* for offsetof */ >>>> +#else >>>> +# include <stddef.h> /* for offsetof */ >>>> +#endif >>> >>> Hello, >>> >>> This patch also solves a warning I encountered when I launched sparse >>> (with make C=1): >>> >>> /usr/lib/gcc-cross/arm-linux-gnueabihf/9/include/stddef.h:406:9: warning: preprocessor token offsetof redefined >>> ./include/linux/stddef.h:17:9: this was the original definition >>> >>> However, I wonder if it useful to keep 'include <stddef.h>' when this file >>> is included from userspace. If it does not compile anymore, there is >>> problem somewhere else, no? >>> >>> (Globally, I always find suspicious a kernel source that includes a system >>> header) >> Hi, >> >> The wireless.h makes use of offsetof and this is defined in system >> stddef.h and in include/linux/stddef.h. When we only include >> linux/stddef.h it will work fine when compiled inside the kernel, but it >> could cause problems when used in user space, because this would include >> ./include/uapi/linux/stddef.h which does not define offsetof. >> >> I think we need the #ifdef __KERNEL__ to make this work in the kernel >> and in the user space. > > In this case, maybe the problem should be resolved directly in > uapi/linux/stddef.h? > > I have found other headers that use offsetof and may have the same problem > (currently, they do not include stddef.h. So, some of the macros do not > compile): > - include/uapi/linux/fuse.h > - include/uapi/linux/genwqe/genwqe_card.h > - include/uapi/linux/gfs2_ondisk.h > Hi, Would you prefer to add this to include/uapi/linux/stddef.h: #ifndef offsetof #define offsetof(TYPE, MEMBER) ((size_t)&((TYPE *)0)->MEMBER) #endif and then include linux/stddef.h in all header files which use offsetof? include/uapi/linux/fuse.h checks for __KERNEL__ and then includes linux/types.h or stdint.h, my initial approach would be to do this in a similar way for stddef.h. What is the kernel policy of including system header files like stddef.h? Hauke
diff --git a/include/uapi/linux/wireless.h b/include/uapi/linux/wireless.h index a2c006a364e0..24f3371ad826 100644 --- a/include/uapi/linux/wireless.h +++ b/include/uapi/linux/wireless.h @@ -74,7 +74,11 @@ #include <linux/socket.h> /* for "struct sockaddr" et al */ #include <linux/if.h> /* for IFNAMSIZ and co... */ -#include <stddef.h> /* for offsetof */ +#ifdef __KERNEL__ +# include <linux/stddef.h> /* for offsetof */ +#else +# include <stddef.h> /* for offsetof */ +#endif /***************************** VERSION *****************************/ /*
When compiling inside the kernel include linux/stddef.h instead of stddef.h. When I compile this header file in backports for power PC I run into a conflict with ptrdiff_t. I was unable to reproduce this in mainline kernel. I still would like to fix this problem in the kernel. Fixes: 6989310f5d43 ("wireless: Use offsetof instead of custom macro.") Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> --- include/uapi/linux/wireless.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)