diff mbox series

[net-next] compat: always include linux/compat.h from net/compat.h

Message ID 20201121175224.1465831-1-kuba@kernel.org (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next] compat: always include linux/compat.h from net/compat.h | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 37 this patch: 37
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 12 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 271 this patch: 271
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Jakub Kicinski Nov. 21, 2020, 5:52 p.m. UTC
We're about to do some reshuffling in networking headers and make
some of the file lose the implicit includes. This results in:

In file included from net/ipv4/netfilter/arp_tables.c:26:
include/net/compat.h:57:23: error: conflicting types for ‘uintptr_t’
 #define compat_uptr_t uintptr_t
                       ^~~~~~~~~
include/asm-generic/compat.h:22:13: note: in expansion of macro ‘compat_uptr_t’
 typedef u32 compat_uptr_t;
             ^~~~~~~~~~~~~
In file included from include/linux/limits.h:6,
                 from include/linux/kernel.h:7,
                 from net/ipv4/netfilter/arp_tables.c:14:
include/linux/types.h:37:24: note: previous declaration of ‘uintptr_t’ was here
 typedef unsigned long  uintptr_t;
                        ^~~~~~~~~

Currently net/compat.h depends on linux/compat.h being included
first. After the upcoming changes this would break the 32bit build.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
Not sure who officially maintains this. Arnd, Christoph any objections?

 include/net/compat.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jakub Kicinski Nov. 21, 2020, 6:40 p.m. UTC | #1
On Sat, 21 Nov 2020 09:52:24 -0800 Jakub Kicinski wrote:
> In file included from net/ipv4/netfilter/arp_tables.c:26:
> include/net/compat.h:57:23: error: conflicting types for ‘uintptr_t’
>  #define compat_uptr_t uintptr_t
>                        ^~~~~~~~~
> include/asm-generic/compat.h:22:13: note: in expansion of macro ‘compat_uptr_t’
>  typedef u32 compat_uptr_t;
>              ^~~~~~~~~~~~~
> In file included from include/linux/limits.h:6,
>                  from include/linux/kernel.h:7,
>                  from net/ipv4/netfilter/arp_tables.c:14:
> include/linux/types.h:37:24: note: previous declaration of ‘uintptr_t’ was here
>  typedef unsigned long  uintptr_t;
>                         ^~~~~~~~~

Ah, damn it, I obviously copied the wrong error into the commit
message. This is the correct one (after removing include of ethtool.h
from netdevice.h):


In file included from ../net/ipv4/netfilter/arp_tables.c:26:
include/net/compat.h:60:40: error: unknown type name ‘compat_uptr_t’; did you mean ‘compat_ptr_ioctl’?
    struct sockaddr __user **save_addr, compat_uptr_t *ptr,
                                        ^~~~~~~~~~~~~
                                        compat_ptr_ioctl
include/net/compat.h:61:4: error: unknown type name ‘compat_size_t’; did you mean ‘compat_sigset_t’?
    compat_size_t *len);
    ^~~~~~~~~~~~~
    compat_sigset_t
Arnd Bergmann Nov. 21, 2020, 9:25 p.m. UTC | #2
On Sat, Nov 21, 2020 at 6:52 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> We're about to do some reshuffling in networking headers and make
> some of the file lose the implicit includes. This results in:
>
> In file included from net/ipv4/netfilter/arp_tables.c:26:
> include/net/compat.h:57:23: error: conflicting types for ‘uintptr_t’
>  #define compat_uptr_t uintptr_t
>                        ^~~~~~~~~
> include/asm-generic/compat.h:22:13: note: in expansion of macro ‘compat_uptr_t’
>  typedef u32 compat_uptr_t;
>              ^~~~~~~~~~~~~
> In file included from include/linux/limits.h:6,
>                  from include/linux/kernel.h:7,
>                  from net/ipv4/netfilter/arp_tables.c:14:
> include/linux/types.h:37:24: note: previous declaration of ‘uintptr_t’ was here
>  typedef unsigned long  uintptr_t;
>                         ^~~~~~~~~
>
> Currently net/compat.h depends on linux/compat.h being included
> first. After the upcoming changes this would break the 32bit build.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> Not sure who officially maintains this. Arnd, Christoph any objections?

Looks good to me. I would actually go one step further and completely
remove this #ifdef, if possible. In the old days, it was not possible to
include linux/compat.h on 32-bit architectures, but now this should just
work without an #ifdef.

     Arnd
Jakub Kicinski Nov. 21, 2020, 9:48 p.m. UTC | #3
On Sat, 21 Nov 2020 22:25:35 +0100 Arnd Bergmann wrote:
> On Sat, Nov 21, 2020 at 6:52 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > We're about to do some reshuffling in networking headers and make
> > some of the file lose the implicit includes. This results in:
> >
> > In file included from net/ipv4/netfilter/arp_tables.c:26:
> > include/net/compat.h:57:23: error: conflicting types for ‘uintptr_t’
> >  #define compat_uptr_t uintptr_t
> >                        ^~~~~~~~~
> > include/asm-generic/compat.h:22:13: note: in expansion of macro ‘compat_uptr_t’
> >  typedef u32 compat_uptr_t;
> >              ^~~~~~~~~~~~~
> > In file included from include/linux/limits.h:6,
> >                  from include/linux/kernel.h:7,
> >                  from net/ipv4/netfilter/arp_tables.c:14:
> > include/linux/types.h:37:24: note: previous declaration of ‘uintptr_t’ was here
> >  typedef unsigned long  uintptr_t;
> >                         ^~~~~~~~~
> >
> > Currently net/compat.h depends on linux/compat.h being included
> > first. After the upcoming changes this would break the 32bit build.
> >
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > ---
> > Not sure who officially maintains this. Arnd, Christoph any objections?  
> 
> Looks good to me. I would actually go one step further and completely
> remove this #ifdef, if possible. In the old days, it was not possible to
> include linux/compat.h on 32-bit architectures, but now this should just
> work without an #ifdef.

Indeed, that appears to work, v2 coming up, thanks!
diff mbox series

Patch

diff --git a/include/net/compat.h b/include/net/compat.h
index 745db0d605b6..08a089bbaecc 100644
--- a/include/net/compat.h
+++ b/include/net/compat.h
@@ -5,10 +5,10 @@ 
 
 struct sock;
 
-#if defined(CONFIG_COMPAT)
-
 #include <linux/compat.h>
 
+#if defined(CONFIG_COMPAT)
+
 struct compat_msghdr {
 	compat_uptr_t	msg_name;	/* void * */
 	compat_int_t	msg_namelen;