* [PATCH] asm-generic/ioctl.h: use BUILD_BUG_ON_ZERO() for type check
@ 2021-02-23 10:06 Masahiro Yamada
2021-02-23 20:04 ` Arnd Bergmann
0 siblings, 1 reply; 4+ messages in thread
From: Masahiro Yamada @ 2021-02-23 10:06 UTC (permalink / raw)
To: Andrew Morton; +Cc: Arnd Bergmann, linux-kernel, Masahiro Yamada, linux-arch
With the latest sparse, I do not see the error claimed by commit
d55875f5d52c ("include/asm-generic/ioctl.h: fix _IOC_TYPECHECK sparse
error").
Anyway, using BUILD_BUG_ON_ZERO() is clearer, and we do not need
to worry about sparse because BUILD_BUG_ON_ZERO() definition in
<linux/build_bug.h> is a constant zero when __CHECKER__ is defined.
Also, remove #ifndef __KERNEL__ from <uapi/asm-generic/ioctl.h>.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
include/asm-generic/ioctl.h | 12 ++++--------
include/uapi/asm-generic/ioctl.h | 13 ++++++-------
2 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/include/asm-generic/ioctl.h b/include/asm-generic/ioctl.h
index 9fda9ed000cd..d5129d70ee1c 100644
--- a/include/asm-generic/ioctl.h
+++ b/include/asm-generic/ioctl.h
@@ -2,17 +2,13 @@
#ifndef _ASM_GENERIC_IOCTL_H
#define _ASM_GENERIC_IOCTL_H
+#include <linux/build_bug.h>
#include <uapi/asm-generic/ioctl.h>
-#ifdef __CHECKER__
-#define _IOC_TYPECHECK(t) (sizeof(t))
-#else
/* provoke compile error for invalid uses of size argument */
-extern unsigned int __invalid_size_argument_for_IOC;
+#undef _IOC_TYPECHECK
#define _IOC_TYPECHECK(t) \
- ((sizeof(t) == sizeof(t[1]) && \
- sizeof(t) < (1 << _IOC_SIZEBITS)) ? \
- sizeof(t) : __invalid_size_argument_for_IOC)
-#endif
+ BUILD_BUG_ON_ZERO(sizeof(t) != sizeof(t[1]) || \
+ sizeof(t) >= (1 << _IOC_SIZEBITS))
#endif /* _ASM_GENERIC_IOCTL_H */
diff --git a/include/uapi/asm-generic/ioctl.h b/include/uapi/asm-generic/ioctl.h
index a84f4db8a250..d50bd39ec3e3 100644
--- a/include/uapi/asm-generic/ioctl.h
+++ b/include/uapi/asm-generic/ioctl.h
@@ -72,9 +72,8 @@
((nr) << _IOC_NRSHIFT) | \
((size) << _IOC_SIZESHIFT))
-#ifndef __KERNEL__
-#define _IOC_TYPECHECK(t) (sizeof(t))
-#endif
+#define _IOC_TYPECHECK(t) 0
+#define _IOC_SIZE_WITH_TYPECHECK(t) (sizeof(t) + _IOC_TYPECHECK(t))
/*
* Used to create numbers.
@@ -82,10 +81,10 @@
* NOTE: _IOW means userland is writing and kernel is reading. _IOR
* means userland is reading and kernel is writing.
*/
-#define _IO(type,nr) _IOC(_IOC_NONE,(type),(nr),0)
-#define _IOR(type,nr,size) _IOC(_IOC_READ,(type),(nr),(_IOC_TYPECHECK(size)))
-#define _IOW(type,nr,size) _IOC(_IOC_WRITE,(type),(nr),(_IOC_TYPECHECK(size)))
-#define _IOWR(type,nr,size) _IOC(_IOC_READ|_IOC_WRITE,(type),(nr),(_IOC_TYPECHECK(size)))
+#define _IO(type,nr) _IOC(_IOC_NONE, type, nr, 0)
+#define _IOR(type,nr,size) _IOC(_IOC_READ, type, nr, _IOC_SIZE_WITH_TYPECHECK(size))
+#define _IOW(type,nr,size) _IOC(_IOC_WRITE, type, nr, _IOC_SIZE_WITH_TYPECHECK(size))
+#define _IOWR(type,nr,size) _IOC(_IOC_READ|_IOC_WRITE, type, nr, _IOC_SIZE_WITH_TYPECHECK(size))
#define _IOR_BAD(type,nr,size) _IOC(_IOC_READ,(type),(nr),sizeof(size))
#define _IOW_BAD(type,nr,size) _IOC(_IOC_WRITE,(type),(nr),sizeof(size))
#define _IOWR_BAD(type,nr,size) _IOC(_IOC_READ|_IOC_WRITE,(type),(nr),sizeof(size))
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] asm-generic/ioctl.h: use BUILD_BUG_ON_ZERO() for type check
2021-02-23 10:06 [PATCH] asm-generic/ioctl.h: use BUILD_BUG_ON_ZERO() for type check Masahiro Yamada
@ 2021-02-23 20:04 ` Arnd Bergmann
2021-02-24 1:57 ` Masahiro Yamada
0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2021-02-23 20:04 UTC (permalink / raw)
To: Masahiro Yamada; +Cc: Andrew Morton, Arnd Bergmann, linux-kernel, linux-arch
On Tue, Feb 23, 2021 at 11:06 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> -#ifdef __CHECKER__
> -#define _IOC_TYPECHECK(t) (sizeof(t))
> -#else
> /* provoke compile error for invalid uses of size argument */
> -extern unsigned int __invalid_size_argument_for_IOC;
> +#undef _IOC_TYPECHECK
> #define _IOC_TYPECHECK(t) \
> - ((sizeof(t) == sizeof(t[1]) && \
> - sizeof(t) < (1 << _IOC_SIZEBITS)) ? \
> - sizeof(t) : __invalid_size_argument_for_IOC)
> -#endif
> + BUILD_BUG_ON_ZERO(sizeof(t) != sizeof(t[1]) || \
> + sizeof(t) >= (1 << _IOC_SIZEBITS))
Using BUILD_BUG_ON_ZERO sounds like a good idea
> #endif /* _ASM_GENERIC_IOCTL_H */
> diff --git a/include/uapi/asm-generic/ioctl.h b/include/uapi/asm-generic/ioctl.h
> index a84f4db8a250..d50bd39ec3e3 100644
> --- a/include/uapi/asm-generic/ioctl.h
> +++ b/include/uapi/asm-generic/ioctl.h
> @@ -72,9 +72,8 @@
> ((nr) << _IOC_NRSHIFT) | \
> ((size) << _IOC_SIZESHIFT))
>
> -#ifndef __KERNEL__
> -#define _IOC_TYPECHECK(t) (sizeof(t))
> -#endif
> +#define _IOC_TYPECHECK(t) 0
> +#define _IOC_SIZE_WITH_TYPECHECK(t) (sizeof(t) + _IOC_TYPECHECK(t))
But I think replacing the #ifndef with an #undef in the other file makes it
harder to understand when reading through it and trying to understand
what it would do when this gets included from kernel and user space.
Arnd
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] asm-generic/ioctl.h: use BUILD_BUG_ON_ZERO() for type check
2021-02-23 20:04 ` Arnd Bergmann
@ 2021-02-24 1:57 ` Masahiro Yamada
2021-02-24 8:41 ` Arnd Bergmann
0 siblings, 1 reply; 4+ messages in thread
From: Masahiro Yamada @ 2021-02-24 1:57 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Andrew Morton, Arnd Bergmann, linux-kernel, linux-arch
On Wed, Feb 24, 2021 at 5:04 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> On Tue, Feb 23, 2021 at 11:06 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> >
> > -#ifdef __CHECKER__
> > -#define _IOC_TYPECHECK(t) (sizeof(t))
> > -#else
> > /* provoke compile error for invalid uses of size argument */
> > -extern unsigned int __invalid_size_argument_for_IOC;
> > +#undef _IOC_TYPECHECK
> > #define _IOC_TYPECHECK(t) \
> > - ((sizeof(t) == sizeof(t[1]) && \
> > - sizeof(t) < (1 << _IOC_SIZEBITS)) ? \
> > - sizeof(t) : __invalid_size_argument_for_IOC)
> > -#endif
> > + BUILD_BUG_ON_ZERO(sizeof(t) != sizeof(t[1]) || \
> > + sizeof(t) >= (1 << _IOC_SIZEBITS))
>
> Using BUILD_BUG_ON_ZERO sounds like a good idea
>
> > #endif /* _ASM_GENERIC_IOCTL_H */
> > diff --git a/include/uapi/asm-generic/ioctl.h b/include/uapi/asm-generic/ioctl.h
> > index a84f4db8a250..d50bd39ec3e3 100644
> > --- a/include/uapi/asm-generic/ioctl.h
> > +++ b/include/uapi/asm-generic/ioctl.h
> > @@ -72,9 +72,8 @@
> > ((nr) << _IOC_NRSHIFT) | \
> > ((size) << _IOC_SIZESHIFT))
> >
> > -#ifndef __KERNEL__
> > -#define _IOC_TYPECHECK(t) (sizeof(t))
> > -#endif
> > +#define _IOC_TYPECHECK(t) 0
> > +#define _IOC_SIZE_WITH_TYPECHECK(t) (sizeof(t) + _IOC_TYPECHECK(t))
>
> But I think replacing the #ifndef with an #undef in the other file makes it
> harder to understand when reading through it and trying to understand
> what it would do when this gets included from kernel and user space.
>
> Arnd
My intention is to improve the UAPI/KAPI decoupling
to decrease the task of scripts/headers_install.sh
Ideally, we could export UAPI headers with
almost no modification.
It is true that scripts/unifdef can remove #ifndef __KERNEL__
blocks, but having the kernel-space code in UAPI headers
does not make sense. Otherwise, our initial motivation
"separate them by directory structure" would be lost.
So, I believe redefining _IOC_TYPECHECK is the right direction.
I can add comments if this is not clear.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] asm-generic/ioctl.h: use BUILD_BUG_ON_ZERO() for type check
2021-02-24 1:57 ` Masahiro Yamada
@ 2021-02-24 8:41 ` Arnd Bergmann
0 siblings, 0 replies; 4+ messages in thread
From: Arnd Bergmann @ 2021-02-24 8:41 UTC (permalink / raw)
To: Masahiro Yamada; +Cc: Andrew Morton, Arnd Bergmann, linux-kernel, linux-arch
On Wed, Feb 24, 2021 at 2:57 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Wed, Feb 24, 2021 at 5:04 AM Arnd Bergmann <arnd@kernel.org> wrote:
> >
> > On Tue, Feb 23, 2021 at 11:06 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> My intention is to improve the UAPI/KAPI decoupling
> to decrease the task of scripts/headers_install.sh
>
> Ideally, we could export UAPI headers with
> almost no modification.
>
> It is true that scripts/unifdef can remove #ifndef __KERNEL__
> blocks, but having the kernel-space code in UAPI headers
> does not make sense. Otherwise, our initial motivation
> "separate them by directory structure" would be lost.
>
> So, I believe redefining _IOC_TYPECHECK is the right direction.
> I can add comments if this is not clear.
Maybe using '#ifndef _IOC_TYPECHECK' would help here?
Another alternative might be to find a way to rewrite the typecheck
macro to make it safe to be used in user space as well, and not
have two different versions.
Arnd
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-02-24 8:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-23 10:06 [PATCH] asm-generic/ioctl.h: use BUILD_BUG_ON_ZERO() for type check Masahiro Yamada
2021-02-23 20:04 ` Arnd Bergmann
2021-02-24 1:57 ` Masahiro Yamada
2021-02-24 8:41 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).