Re: [PATCH]: Make ioctl.h compatible with userland

[Andrew Morton <akpm@linux-foundation.org>]

On Mon, 28 Jul 2008 06:48:52 +0000 (GMT) Michael Abbott <michael@araneidae.co.uk> wrote:

> The attached patch seems to already exist in a number of branches -- it 
> keeps popping up on Google for me, and is certainly already in Debian -- 
> but is strangely absent from mainstream.
> 
> The problem appears to be that the patched file ends up as part of the 
> target toolchain, but unfortunately the gcc constant folding doesn't 
> appear to eliminate the __invalid_size_argument_for_IOC value early 
> enough.  Certainly compiling C++ programs which use _IO... macros as 
> constants fails without this patch.

Could be that `-O0' is associated with the problems.

Plus compilers other than gcc can legitimately use this header.

> No doubt this has been pushed upstream before: this problem seems to date 
> from the very early days of 2.6 ... but here it is again.  It makes sense 
> to do it.
> 
> 
> commit 0df6f37b4e4534f219b5e40cb49ffd9311eb6195
> Author: Michael Abbott <michael.abbott@diamond.ac.uk>
> Date:   Mon Jul 28 07:32:05 2008 +0100
> 
>     Add long established but strangely absent patch to allow ioctl.h to
>     work smoothly with userspace program optimisations.
> 
> diff --git a/include/asm-generic/ioctl.h b/include/asm-generic/ioctl.h
> index 8641813..15828b2 100644
> --- a/include/asm-generic/ioctl.h
> +++ b/include/asm-generic/ioctl.h
> @@ -68,12 +68,16 @@
>  	 ((nr)   << _IOC_NRSHIFT) | \
>  	 ((size) << _IOC_SIZESHIFT))
>  
> +#ifdef __KERNEL__
>  /* provoke compile error for invalid uses of size argument */
>  extern unsigned int __invalid_size_argument_for_IOC;
>  #define _IOC_TYPECHECK(t) \
>  	((sizeof(t) == sizeof(t[1]) && \
>  	  sizeof(t) < (1 << _IOC_SIZEBITS)) ? \
>  	  sizeof(t) : __invalid_size_argument_for_IOC)
> +#else
> +#define _IOC_TYPECHECK(t) (sizeof(t))
> +#endif
>  
>  /* used to create numbers */
>  #define _IO(type,nr)		_IOC(_IOC_NONE,(type),(nr),0)

Gee.

But yes, the patch looks reasonable.

We could also replace that open-coded assertion with the shiny new
BUILD_BUG_ON(), which would a) be cleaner and b) fix the problem which
you describe.  I expect that would be quite safe, but obviously doesn't
have all the testing which the above patch has, so shrug.