Date: Tue, 30 Sep 2003 14:43:13 -0700
From: Andrew Morton <akpm@osdl.org>
To: Russell King <rmk@arm.linux.org.uk>
Cc: arun.sharma@intel.com, linux-kernel@vger.kernel.org, kevin.tian@intel.com,
       willy@debian.org
Subject: Re: [PATCH] incorrect use of sizeof() in ioctl definitions
Message-Id: <20030930144313.601a4655.akpm@osdl.org>
In-Reply-To: <20030930223531.B10154@flint.arm.linux.org.uk>
References: <3F79ED60.2030207@intel.com>
	<20030930140805.0e3158e7.akpm@osdl.org>
	<20030930223531.B10154@flint.arm.linux.org.uk>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org

Russell King <rmk@arm.linux.org.uk> wrote:
>
> >  /* out param: u32*	backlight value: 0 to 15 */
> > -#define FBIO_ATY128_GET_MIRROR	_IOR('@', 1, sizeof(__u32*))
> > +#define FBIO_ATY128_GET_MIRROR	_IOR('@', 1, __u32*)
> >  /* in param: u32*	backlight value: 0 to 15 */
> > -#define FBIO_ATY128_SET_MIRROR	_IOW('@', 2, sizeof(__u32*))
> > +#define FBIO_ATY128_SET_MIRROR	_IOW('@', 2, __u32*)
> >  
> >  static int aty128fb_ioctl(struct inode *inode, struct file *file, u_int cmd,
> >  			  u_long arg, struct fb_info *info)
> > 
> > 
> > Matthew's conversion mainly converted things to size_t, but from the looks
> > of it, __u32* is the right thing to use in this case, I think?
> 
> sizeof(__u32*) may not be sizeof(sizeof(__u32*)), so this would be an API
> change...  Therefore, all these wrong entries need to change to size_t
> (preferably with the real type following inside a comment so we don't
> loose useful information.)

In that case I'm going to need a bit of education as to what the whole
thing is trying to do.

If FBIO_ATY128_SET_MIRROR is really supposed to be passing a pointer into
the ioctl then we *want* the encoded ioctl number to be different for
32-bit-compiled userspace and 64-bit-compiled userspace, don't we?

-
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/