[linux-dvb] Re: refactoring

[Andrew de Quincey <adq_dvb@lidskialf.net>]

On Thursday 07 Oct 2004 17:27, Michael Hunold wrote:
> Hi,
>
> On 07.10.2004 11:07, Andrew de Quincey wrote:
> > Hi, I implemented the function pointer approach for the stv0299. I'm
> > afraid I've decided not to do it, and stick with the ioctl approach.
> > Reasons:
> >
> > 1) The actual switch statement code in the demodulator ioctl() functions
> > is not very big. The reason the ioctl() function may appear big is
> > because many of the demodulators do a lot of the processing directly in
> > the ioctl() function. When moving to a function pointer approach, this
> > code cannot just go away - it has to move into separate functions.
>
> Which is fine IMO.
>
> > 2) Although it does make overriding the ioctl functions nicer, we can
> > already do that with the current code's ioctl_override() (although not
> > quite as nicely).
>
> ioctl_override() is ugly, because it bypasses type safety. IMO the ioctl
> processing should be done completely in the dvb frontend code, not in
> each and every demodulator. By this we can achive guaranteed behaviour.
>
> The same problem exists for other parts of the V3 API, where the API is
> implemented in the driver and not in the dvb core. For example it's
> driver specific if video decoding is automatically stopped when the
> video file descriptor is closed.
>
> I think we have started to walk the road with type safety in mind, so we
> should not stop half way.
>
> > Using a function pointer system also has the disadvantage that is is
> > harder to add in new API calls, or demodulator-specific private calls to
> > support special features.
>
> The driver can always use the driver specific struct dvb_stv0299 (or
> what it is called) to pass any special informations to the driver in a
> typesafe manner when attaching the device. This structure is easily
> extensible.
>
> This information can be honoured when the dvb frontend core calls
> specific functions
>
> > 3) It is quite a lot of work to change over all the demodulators and card
> > drivers. Given the above two points, I just don't think its worth it.
>
> I think it is.

OK, call the ioctl_override()/nontypesafety a temporary solution then.

Can we look into this after the current round of changes? This lot is going to 
be the worst since we're moving from autodetected(ish) frontends to 
explicitly requested ones, and removed most of the spaghetti code. I'd like 
to get the current code out there ASAP so people can start reporting their 
cards don't work any more because of missing frontends/bugs. I don't fancy 
having to deal with two refactoring branches at the same time. 

A move to function pointers can be done later with far less impact since its 
basically just a fairly easy plumbing change.