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

From: Andrew Morton
Date: Wed Jul 30 2008 - 04:11:06 EST


On Mon, 28 Jul 2008 06:48:52 +0000 (GMT) Michael Abbott <michael@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/