| Message ID | 20240828030413.143930-2-xry111@xry111.site (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 439667fb943cfea46d7bde5c7b29c89efec3cbc7 |
| Headers | show |
| Series | mips: Remove posix_types.h include from sigcontext.h | expand |
On Wed, Aug 28, 2024 at 11:04:14AM +0800, Xi Ruoyao wrote: > Nothing in sigcontext.h seems to require anything from > linux/posix_types.h. > > It seems only a relict: in a Linux 2.6.11-rc2 patch [1] the > linux/types.h include was unexplainedly changed to a linux/posix_types.h > include. I can only assume it was just an error. Finally headers_check > complained "found __[us]{8,16,32,64} type without #include > <linux/types.h>" and commit ae612fb05b0f ("headers_check fix: mips, > sigcontext.h") added back the linux/types.h include, but it didn't > remove the posix_types.h include. > > Remove it now. > > [1]:https://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.11-rc2/2.6.11-rc2-mm2/broken-out/mips-generic-mips-updates.patch > > Signed-off-by: Xi Ruoyao <xry111@xry111.site> > --- > arch/mips/include/uapi/asm/sigcontext.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/arch/mips/include/uapi/asm/sigcontext.h b/arch/mips/include/uapi/asm/sigcontext.h > index d0a540e88bb4..d10afd13ee5b 100644 > --- a/arch/mips/include/uapi/asm/sigcontext.h > +++ b/arch/mips/include/uapi/asm/sigcontext.h > @@ -56,7 +56,6 @@ struct sigcontext { > > #if _MIPS_SIM == _MIPS_SIM_ABI64 || _MIPS_SIM == _MIPS_SIM_NABI32 > > -#include <linux/posix_types.h> > /* > * Keep this struct definition in sync with the sigcontext fragment > * in arch/mips/kernel/asm-offsets.c > -- > 2.46.0 applied to mips-next. Thomas.
On Wed, 28 Aug 2024, Xi Ruoyao wrote: > It seems only a relict: in a Linux 2.6.11-rc2 patch [1] the > linux/types.h include was unexplainedly changed to a linux/posix_types.h > include. I can only assume it was just an error. Finally headers_check > complained "found __[us]{8,16,32,64} type without #include > <linux/types.h>" and commit ae612fb05b0f ("headers_check fix: mips, > sigcontext.h") added back the linux/types.h include, but it didn't > remove the posix_types.h include. The original LMO change was this: commit 1dc129df74a76ee16593c9220c3f7289ee627d03 Author: Ralf Baechle <ralf@linux-mips.org> Date: Sat Dec 18 01:09:29 2004 +0000 Don't pull <linux/types.h> into userspace. diff --git a/include/asm-mips/sigcontext.h b/include/asm-mips/sigcontext.h index 844879d63b77..18939e84b6f2 100644 --- a/include/asm-mips/sigcontext.h +++ b/include/asm-mips/sigcontext.h @@ -41,8 +41,6 @@ struct sigcontext { #if _MIPS_SIM == _MIPS_SIM_ABI64 || _MIPS_SIM == _MIPS_SIM_NABI32 -#include <linux/types.h> - /* * Keep this struct definition in sync with the sigcontext fragment * in arch/mips/tools/offset.c @@ -66,6 +64,9 @@ struct sigcontext { }; #ifdef __KERNEL__ + +#include <linux/posix_types.h> + struct sigcontext32 { __u32 sc_regmask; /* Unused */ __u32 sc_status; so not without explanation and clearly to address a userland compilation issue. It seems that the original directive should have just been moved into the __KERNEL__ part without changing the file included though. So commit ae612fb05b0f was wrong too in that it should have just changed the directive in place rather than adding #include <linux/types.h> at the top, as it just brought the issue back that commit 1dc129df74a7 attempted to address. Meanwhile there was commit 269dd2b2526d, which messed things up further, as it should have used `unsigned long long' rather than `__u64' as the 64-bit type (and leave `unsigned int' intact), so commit ae612fb05b0f couldn't actually have done a better fix without correcting commit 269dd2b2526d first. We have since developed UAPI headers, so the issue of the userland use of <linux/types.h> has gone, but still I think we ought to clean up the mess in this header by switching back to standard ISO C `unsigned long long' and `unsigned int' types for members of 64-bit `struct sigcontext', and then the inclusion of <linux/types.h> can go too. That written I think your change makes sense by itself, but I suggest that you update the description and summarise the findings given here. I'm not sure if a copy of the LMO repo is available online at this time though. Maciej
On Wed, 2024-09-04 at 03:06 +0100, Maciej W. Rozycki wrote: > -#include <linux/types.h> > - > /* > * Keep this struct definition in sync with the sigcontext fragment > * in arch/mips/tools/offset.c > @@ -66,6 +64,9 @@ struct sigcontext { > }; > > #ifdef __KERNEL__ > + > +#include <linux/posix_types.h> > + > struct sigcontext32 { > __u32 sc_regmask; /* Unused */ > __u32 sc_status; > > so not without explanation and clearly to address a userland compilation > issue. It seems that the original directive should have just been moved > into the __KERNEL__ part without changing the file included though. Yes, I just mean *changing* the include file is not explained and it seems an error. I'm not familiar with ancient kernels but AFAIK for using __u32 etc. we should use linux/types.h since Linux 1.2. Moving the include into #ifdef __KERNEL__ makes sense to me.
On Wed, 4 Sep 2024, Xi Ruoyao wrote: > Yes, I just mean *changing* the include file is not explained and it > seems an error. I'm not familiar with ancient kernels but AFAIK for > using __u32 etc. we should use linux/types.h since Linux 1.2. As I say the use of __u32, etc. is not needed here in the first place. Maciej
diff --git a/arch/mips/include/uapi/asm/sigcontext.h b/arch/mips/include/uapi/asm/sigcontext.h index d0a540e88bb4..d10afd13ee5b 100644 --- a/arch/mips/include/uapi/asm/sigcontext.h +++ b/arch/mips/include/uapi/asm/sigcontext.h @@ -56,7 +56,6 @@ struct sigcontext { #if _MIPS_SIM == _MIPS_SIM_ABI64 || _MIPS_SIM == _MIPS_SIM_NABI32 -#include <linux/posix_types.h> /* * Keep this struct definition in sync with the sigcontext fragment * in arch/mips/kernel/asm-offsets.c
Nothing in sigcontext.h seems to require anything from linux/posix_types.h. It seems only a relict: in a Linux 2.6.11-rc2 patch [1] the linux/types.h include was unexplainedly changed to a linux/posix_types.h include. I can only assume it was just an error. Finally headers_check complained "found __[us]{8,16,32,64} type without #include <linux/types.h>" and commit ae612fb05b0f ("headers_check fix: mips, sigcontext.h") added back the linux/types.h include, but it didn't remove the posix_types.h include. Remove it now. [1]:https://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.11-rc2/2.6.11-rc2-mm2/broken-out/mips-generic-mips-updates.patch Signed-off-by: Xi Ruoyao <xry111@xry111.site> --- arch/mips/include/uapi/asm/sigcontext.h | 1 - 1 file changed, 1 deletion(-)