On Mon, Nov 05, 2007, Steven Toth wrote:
> Johannes Stezenbach wrote:
>>
>> struct dvb_tuning_param p[3] = {
>> { .id = MODULATION, .val = MOD_8VSB },
>> { .id = FREQUENCY, .val = 591250000 },
>> { .id = 0 }
>> };
>> ioctl(fd, DVB_TUNE, p);
>
>
> You can't reliably pass in variably length arrays into the kernel, or none
> of the other kernel wide drivers do, they need to be fixed length for
> sanity reasons.
Of course you can have variable length args to ioctl(). It's
just that you can't let dvb_usercopy() do the work anymore but
have to call copy_from_user() yourself, but I would favor a simple,
generic API anytime over one with unnecessary, arbitrary
limits, so IMHO it's worth the little extra effort.
#define DVB_TUNE _IOC(_IOC_WRITE,'o',82,0)
plus
diff -r 1acfe4149714 linux/drivers/media/dvb/dvb-core/dvbdev.c
--- a/linux/drivers/media/dvb/dvb-core/dvbdev.c Mon Nov 05 10:30:39 2007 -0200
+++ b/linux/drivers/media/dvb/dvb-core/dvbdev.c Mon Nov 05 18:23:41 2007 +0100
@@ -362,9 +362,11 @@ int dvb_usercopy(struct inode *inode, st
case _IOC_READ: /* some v4l ioctls are marked wrong ... */
case _IOC_WRITE:
case (_IOC_WRITE | _IOC_READ):
- if (_IOC_SIZE(cmd) <= sizeof(sbuf)) {
+ if (_IOC_SIZE(cmd) == 0)
+ parg = arg;
+ else if (_IOC_SIZE(cmd) <= sizeof(sbuf))
parg = sbuf;
- } else {
+ else {
/* too big to allocate from stack */
mbuf = kmalloc(_IOC_SIZE(cmd),GFP_KERNEL);
if (NULL == mbuf)
(untested)
(BTW, the majority of ioctls don't encode the argument size, this
feature was invented just a few years ago; see man ioctl_list)
Or you could encode the lenght seperately like e.g.
I2C_RDWR does with its struct i2c_rdwr_ioctl_data argument.
It's a matter of taste, not sanity or security.
> 1) I've made minor changes to dvb_core to interpret these messages and
> handle the ioctl. No changes have been made to the frontend drivers.
>
> 2) Adding support for s2 _inside_ the kernel will mean adding a single
> method to dvb_frontend_ops which allows the dvb_core to notify a new S2
> driver. The goal here is to minimize these changes also. I haven't
> demonstrated that code here, becuse I felt it was more important to show
> the impact to the userland API/ABI, knowing that DVB-S2 and other advanced
> types (including analog) would naturally fit well within this model.
>
> 3) This applies to all devs. I welcome all feedback, for sure the sytax
> might need some cleanup, but please don't shoot the idea down when it's
> only had 6-8 hours work of engineering behind it. It's proof of concept. It
> doesn't contain any of Manu's code so I don't feel bound politically or
> technically. I think given another 4-5 hours I could show the HVR4000
> working, and demonstrate many of the userland signal/statistic API's.
>
> At this stage I'm looking for a "Yes, in principle we like the idea, but
> show me how you do feature XYZ" from other devs. At which point I'll flush
> out more code this would probably lead to an RFC for your approval.
Seems like no one is interested.
BTW, since every DVB-S2 demod is also a DVB-S demod, why does
no one split the DVB-S parts of their driver for merging
first? It would make the users happy as it would change the
state from "card not supported" to "card works for 95% of existing
transponders". (Dunno how this fits into this thread but...)
Johannes
_______________________________________________
linux-dvb mailing list
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
