Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210223100619.798698-1-masahiroy@kernel.org>
In-Reply-To: <20210223100619.798698-1-masahiroy@kernel.org>
From:   Arnd Bergmann <arnd@kernel.org>
Date:   Tue, 23 Feb 2021 21:04:10 +0100
Message-ID: <CAK8P3a1b5Tr8Gt_DcUq9JQj4G6O5ZHf44P2ZdYZRGQY8iPs43Q@mail.gmail.com>
Subject: Re: [PATCH] asm-generic/ioctl.h: use BUILD_BUG_ON_ZERO() for type check
To:     Masahiro Yamada <masahiroy@kernel.org>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Arnd Bergmann <arnd@arndb.de>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        linux-arch <linux-arch@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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