On Monday 05 November 2007, Johannes Stezenbach wrote:
> 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...)
> 

hi everybody,

what are we doing now?

imho we are discussing a new change to the dvb-s2 api again. 
although i do like this ioctl approach for further enhancements of the api, 
i must admit that things that are already done have to be done again.

iirc, there is a multiproto version of szap, vdr, ...

although, existing drivers e.g. stb0899 must be rewritten/patched to handle the
new iotcl. 

at least i do not know what this will mean to the rework and test scenario, 
on both sides. this means if we take multiproto from manu - what has steven got 
to do,
and, if we take stevens approach, whats on manus side.

i would really appreciate to get a working compromise instead of starting this 
api extension
all over again for the X time since Nov. 2005 when - iirc felix domke and me - 
started the discussion.


"Yes, in principle I like the idea" but someday we must get an end to the 
implementation and 
merge it.

best regards
marcel





_______________________________________________
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

Reply via email to