Em 23-05-2011 12:32, Andreas Oberritter escreveu: > On 05/23/2011 04:51 PM, Hans Petter Selasky wrote: >> On Monday 23 May 2011 16:37:18 Andreas Oberritter wrote: >>> On 05/23/2011 03:58 PM, Hans Petter Selasky wrote: >>>> From be7d0f72ebf4d945cfb2a5c9cc871707f72e1e3c Mon Sep 17 00:00:00 2001 >>>> From: Hans Petter Selasky <hselasky@xxxxxxx> >>>> Date: Mon, 23 May 2011 15:56:31 +0200 >>>> Subject: [PATCH] FE_GET_PROPERTY should be _IOW, because the associated >>>> structure is transferred from userspace to kernelspace. Keep the old >>>> ioctl around for compatibility so that existing code is not broken. >>> >> >> Hi, >> >>> Good catch, but I think _IOWR would be right, because the result gets >>> copied from kernelspace to userspace. >> >> Those flags are only for the IOCTL associated structure itself. The V4L DVB >> kernel only reads the dtv_properties structure in either case and does not >> write any data back to it. That's why only _IOW is required. > > I see. > >> I checked somewhat and the R/W bits in the IOCTL command does not appear do be >> matched to the R/W permissions you have on the file handle? Or am I mistaken? > > You're right. There's no direct relationship between them, at least not > within dvb-core. > >> In other words the IOCTL R/W (_IOC_READ, _IOC_WRITE) bits should not reflect >> what the IOCTL actually does, like modifying indirect data? > > I'm not sure. Your patch is certainly doing the right thing for the > current implementation of dvb_usercopy, which however wasn't designed > with variable length arrays in mind. The dvb_usercopy will do the right thing, if we use _IOR or _IORW. > Taking dvb_usercopy aside, my interpretation of the ioctl bits was: > - _IOC_READ is required if copy_to_user/put_user needs to be used during > the ioctl. > - _IOC_WRITE is required if copy_from_user/get_user needs to be used > during the ioctl. That is my understanding too. I agree that _IOWR seems to be the more appropriate definition for it. That's said, this is just a naming convention. Kernel core won't enforce any special behavior, as there are some violations about this convention on a few places. > > Whether that's limited to the structure directly encoded in the ioctl or > not is unclear to me. Maybe someone at LKML can shed some light on that. I prefer to not apply this patch, as it won't fix anything. Adding an _OLD means that we'll need later to remove it, causing a regression. Ok, we may do like we did with V4L _OLD ioctl's that were marked as _OLD at 2.6.5 and were removed on a late 2.6.3x. Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
