[linux-dvb] DVB-S2 API vs. HVR4000: When?

Johannes Stezenbach js at linuxtv.org
Mon Nov 5 18:37:50 CET 2007


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



More information about the linux-dvb mailing list