--- ia64-linux2.6.0/include/linux/matroxfb.h Tue Aug 26 16:47:26 2003
+++ ia64-linux2.6.0-patch/include/linux/matroxfb.h Fri Sep 5 16:24:58 2003
@@ -15,21 +15,21 @@
#define MATROXFB_OUTPUT_MODE_NTSC 0x0002
#define MATROXFB_OUTPUT_MODE_MONITOR 0x0080
};
-#define MATROXFB_SET_OUTPUT_MODE _IOW('n',0xFA,sizeof(struct matroxioc_output_mode))
-#define MATROXFB_GET_OUTPUT_MODE _IOWR('n',0xFA,sizeof(struct matroxioc_output_mode))
+#define MATROXFB_SET_OUTPUT_MODE _IOW('n',0xFA,struct matroxioc_output_mode)
+#define MATROXFB_GET_OUTPUT_MODE _IOWR('n',0xFA,struct matroxioc_output_mode)
/* bitfield */
#define MATROXFB_OUTPUT_CONN_PRIMARY (1 << MATROXFB_OUTPUT_PRIMARY)
#define MATROXFB_OUTPUT_CONN_SECONDARY (1 << MATROXFB_OUTPUT_SECONDARY)
#define MATROXFB_OUTPUT_CONN_DFP (1 << MATROXFB_OUTPUT_DFP)
/* connect these outputs to this framebuffer */
-#define MATROXFB_SET_OUTPUT_CONNECTION _IOW('n',0xF8,sizeof(__u32))
+#define MATROXFB_SET_OUTPUT_CONNECTION _IOW('n',0xF8,__u32)
/* which outputs are connected to this framebuffer */
-#define MATROXFB_GET_OUTPUT_CONNECTION _IOR('n',0xF8,sizeof(__u32))
+#define MATROXFB_GET_OUTPUT_CONNECTION _IOR('n',0xF8,__u32)
/* which outputs are available for this framebuffer */
-#define MATROXFB_GET_AVAILABLE_OUTPUTS _IOR('n',0xF9,sizeof(__u32))
+#define MATROXFB_GET_AVAILABLE_OUTPUTS _IOR('n',0xF9,__u32)
/* which outputs exist on this framebuffer */
-#define MATROXFB_GET_ALL_OUTPUTS _IOR('n',0xFB,sizeof(__u32))
+#define MATROXFB_GET_ALL_OUTPUTS _IOR('n',0xFB,__u32)
enum matroxfb_ctrl_id {
MATROXFB_CID_TESTOUT = V4L2_CID_PRIVATE_BASE,
--- ia64-linux2.6.0/drivers/video/aty/aty128fb.c Fri Sep 5 16:10:59 2003
+++ ia64-linux2.6.0-patch/drivers/video/aty/aty128fb.c Fri Sep 5 16:01:43 2003
@@ -2041,9 +2041,9 @@
#define ATY_MIRROR_CRT_ON 0x00000002
/* 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)
--- ia64-linux2.6.0/include/linux/coda.h Tue Aug 26 16:47:26 2003
+++ ia64-linux2.6.0-patch/include/linux/coda.h Fri Sep 5 16:15:54 2003
@@ -324,7 +324,7 @@
#define VC_MAXMSGSIZE sizeof(union inputArgs)+sizeof(union outputArgs) +\
VC_MAXDATASIZE
-#define CIOC_KERNEL_VERSION _IOWR('c', 10, sizeof (int))
+#define CIOC_KERNEL_VERSION _IOWR('c', 10, int)
#if 0
#define CODA_KERNEL_VERSION 0 /* don't care about kernel version number */
--- ia64-linux2.6.0/drivers/net/wan/cosa.h Tue Aug 26 16:47:25 2003
+++ ia64-linux2.6.0-patch/drivers/net/wan/cosa.h Fri Sep 5 16:17:52 2003
@@ -73,19 +73,19 @@
#define COSAIORSET _IO('C',0xf0)
/* Start microcode at given address */
-#define COSAIOSTRT _IOW('C',0xf1,sizeof(int))
+#define COSAIOSTRT _IOW('C',0xf1,int)
/* Read the block from the device memory */
-#define COSAIORMEM _IOR('C',0xf2,sizeof(struct cosa_download *))
+#define COSAIORMEM _IOR('C',0xf2,struct cosa_download*)
/* Write the block to the device memory (i.e. download the microcode) */
-#define COSAIODOWNLD _IOW('C',0xf2,sizeof(struct cosa_download *))
+#define COSAIODOWNLD _IOW('C',0xf2,struct cosa_download*)
/* Read the device type (one of "srp", "cosa", and "cosa8" for now) */
-#define COSAIORTYPE _IOR('C',0xf3,sizeof(char *))
+#define COSAIORTYPE _IOR('C',0xf3,char*)
/* Read the device identification string */
-#define COSAIORIDSTR _IOR('C',0xf4,sizeof(char *))
+#define COSAIORIDSTR _IOR('C',0xf4,char*)
/* Maximum length of the identification string. */
#define COSA_MAX_ID_STRING 128
@@ -100,7 +100,7 @@
#define COSAIONRCHANS _IO('C',0xf8)
/* Set the driver for the bus-master operations */
-#define COSAIOBMSET _IOW('C', 0xf9, sizeof(unsigned short))
+#define COSAIOBMSET _IOW('C', 0xf9, unsigned short)
#define COSA_BM_OFF 0 /* Bus-mastering off - use ISA DMA (default) */
#define COSA_BM_ON 1 /* Bus-mastering on - faster but untested */
--- ia64-linux2.6.0/include/linux/fs.h Fri Sep 5 16:14:19 2003
+++ ia64-linux2.6.0-patch/include/linux/fs.h Fri Sep 5 16:18:57 2003
@@ -187,15 +187,15 @@
#define BLKSSZGET _IO(0x12,104)/* get block device sector size */
#if 0
#define BLKPG _IO(0x12,105)/* See blkpg.h */
-#define BLKELVGET _IOR(0x12,106,sizeof(blkelv_ioctl_arg_t))/* elevator get */
-#define BLKELVSET _IOW(0x12,107,sizeof(blkelv_ioctl_arg_t))/* elevator set */
+#define BLKELVGET _IOR(0x12,106,blkelv_ioctl_arg_t)/* elevator get */
+#define BLKELVSET _IOW(0x12,107,blkelv_ioctl_arg_t)/* elevator set */
/* This was here just to show that the number is taken -
probably all these _IO(0x12,*) ioctls should be moved to blkpg.h. */
#endif
/* A jump here: 108-111 have been used for various private purposes. */
-#define BLKBSZGET _IOR(0x12,112,sizeof(int))
-#define BLKBSZSET _IOW(0x12,113,sizeof(int))
-#define BLKGETSIZE64 _IOR(0x12,114,sizeof(u64)) /* return device size in bytes (u64 *arg) */
+#define BLKBSZGET _IOR(0x12,112,int)
+#define BLKBSZSET _IOW(0x12,113,int)
+#define BLKGETSIZE64 _IOR(0x12,114,u64) /* return device size in bytes (u64 *arg) */
#define BMAP_IOCTL 1 /* obsolete - kept for compatibility */
#define FIBMAP _IO(0x00,1) /* bmap access */
--- ia64-linux2.6.0/include/linux/i8k.h Tue Aug 26 16:47:26 2003
+++ ia64-linux2.6.0-patch/include/linux/i8k.h Mon Sep 8 09:25:39 2003
@@ -22,11 +22,11 @@
#define I8K_BIOS_VERSION _IOR ('i', 0x80, 4)
#define I8K_MACHINE_ID _IOR ('i', 0x81, 16)
-#define I8K_POWER_STATUS _IOR ('i', 0x82, sizeof(int))
-#define I8K_FN_STATUS _IOR ('i', 0x83, sizeof(int))
-#define I8K_GET_TEMP _IOR ('i', 0x84, sizeof(int))
-#define I8K_GET_SPEED _IOWR('i', 0x85, sizeof(int))
-#define I8K_GET_FAN _IOWR('i', 0x86, sizeof(int))
+#define I8K_POWER_STATUS _IOR ('i', 0x82, int)
+#define I8K_FN_STATUS _IOR ('i', 0x83, int)
+#define I8K_GET_TEMP _IOR ('i', 0x84, int)
+#define I8K_GET_SPEED _IOWR('i', 0x85, int)
+#define I8K_GET_FAN _IOWR('i', 0x86, int)
#define I8K_SET_FAN _IOWR('i', 0x87, sizeof(int)*2)
#define I8K_FAN_LEFT 1
--- ia64-linux2.6.0/include/linux/if_pppox.h Tue Aug 26 16:47:26 2003
+++ ia64-linux2.6.0-patch/include/linux/if_pppox.h Fri Sep 5 16:22:06 2003
@@ -67,9 +67,9 @@
*
********************************************************************/
-#define PPPOEIOCSFWD _IOW(0xB1 ,0, sizeof(struct sockaddr_pppox))
+#define PPPOEIOCSFWD _IOW(0xB1 ,0, struct sockaddr_pppox)
#define PPPOEIOCDFWD _IO(0xB1 ,1)
-/*#define PPPOEIOCGFWD _IOWR(0xB1,2, sizeof(struct sockaddr_pppox))*/
+/*#define PPPOEIOCGFWD _IOWR(0xB1,2, struct sockaddr_pppox)*/
/* Codes to identify message types */
#define PADI_CODE 0x09
--- ia64-linux2.6.0/include/linux/pmu.h Tue Aug 26 16:47:26 2003
+++ ia64-linux2.6.0-patch/include/linux/pmu.h Fri Sep 5 16:26:16 2003
@@ -107,15 +107,15 @@
/* no param */
#define PMU_IOC_SLEEP _IO('B', 0)
/* out param: u32* backlight value: 0 to 15 */
-#define PMU_IOC_GET_BACKLIGHT _IOR('B', 1, sizeof(__u32*))
+#define PMU_IOC_GET_BACKLIGHT _IOR('B', 1, __u32*)
/* in param: u32 backlight value: 0 to 15 */
-#define PMU_IOC_SET_BACKLIGHT _IOW('B', 2, sizeof(__u32))
+#define PMU_IOC_SET_BACKLIGHT _IOW('B', 2, __u32)
/* out param: u32* PMU model */
-#define PMU_IOC_GET_MODEL _IOR('B', 3, sizeof(__u32*))
+#define PMU_IOC_GET_MODEL _IOR('B', 3, __u32*)
/* out param: u32* has_adb: 0 or 1 */
-#define PMU_IOC_HAS_ADB _IOR('B', 4, sizeof(__u32*))
+#define PMU_IOC_HAS_ADB _IOR('B', 4, __u32*)
/* out param: u32* can_sleep: 0 or 1 */
-#define PMU_IOC_CAN_SLEEP _IOR('B', 5, sizeof(__u32*))
+#define PMU_IOC_CAN_SLEEP _IOR('B', 5, __u32*)
/* no param */
#define PMU_IOC_GRAB_BACKLIGHT _IOR('B', 6, 0)
--- ia64-linux2.6.0/include/linux/radeonfb.h Tue Aug 26 16:47:26 2003
+++ ia64-linux2.6.0-patch/include/linux/radeonfb.h Fri Sep 5 16:26:41 2003
@@ -8,8 +8,8 @@
#define ATY_RADEON_CRT_ON 0x00000002
-#define FBIO_RADEON_GET_MIRROR _IOR('@', 3, sizeof(__u32*))
-#define FBIO_RADEON_SET_MIRROR _IOW('@', 4, sizeof(__u32*))
+#define FBIO_RADEON_GET_MIRROR _IOR('@', 3, __u32*)
+#define FBIO_RADEON_SET_MIRROR _IOW('@', 4, __u32*)
#endif
--- ia64-linux2.6.0/include/video/sisfb.h Tue Aug 26 16:47:26 2003
+++ ia64-linux2.6.0-patch/include/video/sisfb.h Fri Sep 5 16:27:27 2003
@@ -145,9 +145,9 @@
/* If changing this, vgatypes.h must also be changed (for X driver) */
/* TW: ioctl for identifying and giving some info (esp. memory heap start) */
-#define SISFB_GET_INFO _IOR('n',0xF8,sizeof(__u32))
+#define SISFB_GET_INFO _IOR('n',0xF8,__u32)
-#define SISFB_GET_VBRSTATUS _IOR('n',0xF9,sizeof(__u32))
+#define SISFB_GET_VBRSTATUS _IOR('n',0xF9,__u32)
/* TW: Structure argument for SISFB_GET_INFO ioctl */
typedef struct _SISFB_INFO sisfb_info, *psisfb_info;
Arun Sharma <[email protected]> wrote:
>
> Some drivers seem to use macros such as _IOR/_IOW in a way that ends up calling the sizeof() operator twice. For eg:
>
> -#define FBIO_ATY128_GET_MIRROR _IOR('@', 1, sizeof(__u32*))
> +#define FBIO_ATY128_GET_MIRROR _IOR('@', 1, __u32*)
>
> from include/asm-ia64/ioctl.h (other archs are similar):
>
> #define _IOR(type,nr,size) _IOC(_IOC_READ,(type),(nr),sizeof(size))
Matthew Wilcox fixed all except one of these a while back. What is left
over is this chunk:
diff -puN drivers/video/aty/aty128fb.c~sizeof-in-ioctl-fix drivers/video/aty/aty128fb.c
--- 25/drivers/video/aty/aty128fb.c~sizeof-in-ioctl-fix Tue Sep 30 14:04:12 2003
+++ 25-akpm/drivers/video/aty/aty128fb.c Tue Sep 30 14:04:12 2003
@@ -2041,9 +2041,9 @@ aty128fb_setcolreg(u_int regno, u_int re
#define ATY_MIRROR_CRT_ON 0x00000002
/* 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?
On Tue, Sep 30, 2003 at 02:08:05PM -0700, Andrew Morton wrote:
> diff -puN drivers/video/aty/aty128fb.c~sizeof-in-ioctl-fix drivers/video/aty/aty128fb.c
> --- 25/drivers/video/aty/aty128fb.c~sizeof-in-ioctl-fix Tue Sep 30 14:04:12 2003
> +++ 25-akpm/drivers/video/aty/aty128fb.c Tue Sep 30 14:04:12 2003
> @@ -2041,9 +2041,9 @@ aty128fb_setcolreg(u_int regno, u_int re
> #define ATY_MIRROR_CRT_ON 0x00000002
>
> /* 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.)
--
Russell King ([email protected]) http://www.arm.linux.org.uk/personal/
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core
On Tue, Sep 30, 2003 at 11:25:56PM +0100, Matthew Wilcox wrote:
> On Tue, Sep 30, 2003 at 02:08:05PM -0700, Andrew Morton wrote:
> > Arun Sharma <[email protected]> wrote:
> > >
> > > Some drivers seem to use macros such as _IOR/_IOW in a way that ends up
> > > calling the sizeof() operator twice. For eg:
> > >
> > > -#define FBIO_ATY128_GET_MIRROR _IOR('@', 1, sizeof(__u32*))
> > > +#define FBIO_ATY128_GET_MIRROR _IOR('@', 1, __u32*)
But this changes the define. You want
#define FBIO_ATY128_GET_MIRROR _IOR_BAD('@', 1, __u32*)
> +#define _IOR(type,nr,size) _IOC(_IOC_READ,(type),(nr),(_IOC_TYPECHECK(size)
> ))
> +#define _IOR_BAD(type,nr,size) _IOC(_IOC_READ,(type),(nr),sizeof(size))
Something else we should do is to change all occurrences of 'size'
here into 'argtype'. All this nonsense came because of the bad choice
of identifier.
On Tue, Sep 30, 2003 at 02:08:05PM -0700, Andrew Morton wrote:
> Arun Sharma <[email protected]> wrote:
> >
> > Some drivers seem to use macros such as _IOR/_IOW in a way that ends up calling the sizeof() operator twice. For eg:
> >
> > -#define FBIO_ATY128_GET_MIRROR _IOR('@', 1, sizeof(__u32*))
> > +#define FBIO_ATY128_GET_MIRROR _IOR('@', 1, __u32*)
The real problem is that ia64 doesn't have the typechecking ioctl
definitions yet. Here's what I committed for parisc recently (it applies
to ia64 too):
+++ include/asm-parisc/ioctl.h 21 Sep 2003 01:01:38 -0000 1.2
@@ -44,11 +44,21 @@
((nr) << _IOC_NRSHIFT) | \
((size) << _IOC_SIZESHIFT))
+/* provoke compile error for invalid uses of size argument */
+extern 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)
+
/* used to create numbers */
#define _IO(type,nr) _IOC(_IOC_NONE,(type),(nr),0)
-#define _IOR(type,nr,size) _IOC(_IOC_READ,(type),(nr),sizeof(size))
-#define _IOW(type,nr,size) _IOC(_IOC_WRITE,(type),(nr),sizeof(size))
-#define _IOWR(type,nr,size) _IOC(_IOC_READ|_IOC_WRITE,(type),(nr),sizeof(siz
e))
+#define _IOR(type,nr,size) _IOC(_IOC_READ,(type),(nr),(_IOC_TYPECHECK(size)
))
+#define _IOW(type,nr,size) _IOC(_IOC_WRITE,(type),(nr),(_IOC_TYPECHECK(size
)))
+#define _IOWR(type,nr,size) _IOC(_IOC_READ|_IOC_WRITE,(type),(nr),(_IOC_TYPE
CHECK(size)))
+#define _IOR_BAD(type,nr,size) _IOC(_IOC_READ,(type),(nr),sizeof(size))
+#define _IOW_BAD(type,nr,size) _IOC(_IOC_WRITE,(type),(nr),sizeof(size))
+#define _IOWR_BAD(type,nr,size) _IOC(_IOC_READ|_IOC_WRITE,(type),(nr),si
zeof(size))
/* used to decode ioctl numbers.. */
#define _IOC_DIR(nr) (((nr) >> _IOC_DIRSHIFT) & _IOC_DIRMASK)
> Matthew Wilcox fixed all except one of these a while back. What is left
> over is this chunk:
>
>
> diff -puN drivers/video/aty/aty128fb.c~sizeof-in-ioctl-fix drivers/video/aty/aty128fb.c
> --- 25/drivers/video/aty/aty128fb.c~sizeof-in-ioctl-fix Tue Sep 30 14:04:12 2003
> +++ 25-akpm/drivers/video/aty/aty128fb.c Tue Sep 30 14:04:12 2003
> @@ -2041,9 +2041,9 @@ aty128fb_setcolreg(u_int regno, u_int re
> #define ATY_MIRROR_CRT_ON 0x00000002
>
> /* 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?
I think so, given:
radeonfb.h:#define FBIO_RADEON_GET_MIRROR _IOR('@', 3, size_t)
radeonfb.h:#define FBIO_RADEON_SET_MIRROR _IOW('@', 4, size_t)
Though we can also change it to _IOR_BAD and _IOW_BAD.
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
> > 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.)
I hit this bug a while back - most often it's for 4byte sized objects
(ints or pointers) on x86 arch (don't know about others). Unfortunately
there was a case where it was for an 8 and was wrongly declared via sizeof
and thus 4. I'd suggest moving all to single sizeof's and where this
causes API changes to implement bogus/extra declarations which could then
possibly be faded out with time. However currently the glibc kernel
headers are also similarly screwed. Obviously this would bloat the kernel
a little, OTOH for x86 arch I only noticed one case where it didn't end up
the same whether thru double or single sizeof. Perhaps we could be lucky
and have almost as much luck on other archs?
MaZe.
Russell King <[email protected]> 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?