Messages in this thread Patch in this message | | | Date | Wed, 30 Jul 2008 01:10:35 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH]: Make ioctl.h compatible with userland |
| |
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.
| |