Re: video_usercopy() enforces change of VideoText IOCTLs since 2.6.8

[linux@MichaelGeng.de (Michael Geng)]

On Mon, Oct 11, 2004 at 05:14:55PM +0200, Gerd Knorr wrote:
> > +#define VTXIOCGETINFO	_IOR  (0x81,  1, vtx_info_t)
> > +#define VTXIOCCLRPAGE	_IOW  (0x81,  2, vtx_pagereq_t)
> > +#define VTXIOCCLRFOUND	_IOW  (0x81,  3, vtx_pagereq_t)
> > +#define VTXIOCPAGEREQ	_IOW  (0x81,  4, vtx_pagereq_t)
> > +#define VTXIOCGETSTAT	_IOW  (0x81,  5, vtx_pagereq_t)
> > +#define VTXIOCGETPAGE	_IOW  (0x81,  6, vtx_pagereq_t)
> > +#define VTXIOCSTOPDAU	_IOW  (0x81,  7, vtx_pagereq_t)
> 
> Hmm, _IOW for VTXIOCGET* looks bogous, is that really correct?

This is the definition of the argument to the VTXIOCGET* IOCTLs:

typedef struct 
{
	int page;	/* number of requested page (hexadecimal) */
	int hour;	/* requested hour (hexadecimal) */
	int minute;	/* requested minute (hexadecimal) */
	int pagemask;	/* mask defining which values of the above are set */
	int pgbuf;	/* buffer where page will be stored */
	int start;	/* start of requested part of page */
	int end;	/* end of requested part of page */
	void __user *buffer;	/* pointer to beginning of destination buffer */
}
vtx_pagereq_t;

The driver returns all data in the buffer field. Copying there is done by a seperate call to
copy_to_user(). All other fields are never changed by the driver. So the _IOW definition is ok.

> Note that you often need RW for read/get ioctls because even these
> often pass data to the driver as well (for example the vtx page number
> you want query the status for).  Please double-check that.  Otherwise
> the patch looks ok to me.

Thank you! Andrew, could you please forward the patch? 
Suggestion for a comment line:
Videotext: IOCTLs changed to match _IO macros in linux/ioctl.h

Michael