Re: [PATCH] incorrect use of sizeof() in ioctl definitions

[Andrew Morton <akpm@osdl.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?