[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20080730011035.a7d84e55.akpm@linux-foundation.org>
Date: Wed, 30 Jul 2008 01:10:35 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: Michael Abbott <michael@...neidae.co.uk>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH]: Make ioctl.h compatible with userland
On Mon, 28 Jul 2008 06:48:52 +0000 (GMT) Michael Abbott <michael@...neidae.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@...mond.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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists