Date: Wed, 30 Jul 2008 01:10:35 -0700
From: Andrew Morton <akpm@linux-foundation.org>
To: Michael Abbott <michael@araneidae.co.uk>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH]: Make ioctl.h compatible with userland
Message-Id: <20080730011035.a7d84e55.akpm@linux-foundation.org>
In-Reply-To: <20080728063957.J72435@saturn.araneidae.co.uk>
References: <20080728063957.J72435@saturn.araneidae.co.uk>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2477
Lines: 65

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.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/