DVB: BANDWIDTH_TO_KHZ strangeness

DVB: BANDWIDTH_TO_KHZ strangeness

[Adrian Bunk]

drivers/media/dvb/frontends/dibx000_common.h contains:

<--  snip  -->

...
#define BANDWIDTH_TO_KHZ(v) ( (v) == BANDWIDTH_8_MHZ  ? 8000 : \
                             (v) == BANDWIDTH_7_MHZ  ? 7000 : \
                             (v) == BANDWIDTH_6_MHZ  ? 6000 : 8000 )
...

<--  snip  -->


Commit b6884a17fc70e979ef34e4b5560988b522bb50a0 added to both of 
drivers/media/dvb/frontends/dib7000{m,p}.c:

<--  snip  -->

...
        factor = BANDWIDTH_TO_KHZ(ch->u.ofdm.bandwidth);
        if (factor >= 5000)
                factor = 1;
        else
                factor = 6;
...

<--  snip  -->


factor < 5000 is obviously never possible.

drivers/media/dvb/frontends/dib0070.c contains a similar assumption that 
BANDWIDTH_TO_KHZ() could result in values other than {6,7,8}000
(I haven't checked whether there are more such assumptions in other 
places).

Spotted by the Coverity checker.


cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [v4l-dvb-maintainer] DVB: BANDWIDTH_TO_KHZ strangeness

[Patrick Boettcher]

Hi all,

This coverity checker is great. He even finds all the things which are 
there because I'm lazy ;) .

The dibXXXX-drivers are all generated from our internal drivers to have at 
least something for Linux. Because of no time there is no dedicated 
source for the kernel but just the processed one.

The particular issue here is, that the internal architecture supports a 
variable bandwidth while the linux-dvb architecture only has 3 fixed 
values. 

I know that in the future the linux-dvb-API will also support other 
bandwidths so I'm begging for mercy for those 3 things here to not get too 
much out-of-sync with our internal code.

thanks,
Patrick.

On Sun, 14 Oct 2007, Adrian Bunk wrote:
> drivers/media/dvb/frontends/dibx000_common.h contains:
> 
> <--  snip  -->
> 
> ...
> #define BANDWIDTH_TO_KHZ(v) ( (v) == BANDWIDTH_8_MHZ  ? 8000 : \
>                              (v) == BANDWIDTH_7_MHZ  ? 7000 : \
>                              (v) == BANDWIDTH_6_MHZ  ? 6000 : 8000 )
> ...
> 
> <--  snip  -->
> 
> 
> Commit b6884a17fc70e979ef34e4b5560988b522bb50a0 added to both of 
> drivers/media/dvb/frontends/dib7000{m,p}.c:
> 
> <--  snip  -->
> 
> ...
>         factor = BANDWIDTH_TO_KHZ(ch->u.ofdm.bandwidth);
>         if (factor >= 5000)
>                 factor = 1;
>         else
>                 factor = 6;
> ...
> 
> <--  snip  -->
> 
> 
> factor < 5000 is obviously never possible.
> 
> drivers/media/dvb/frontends/dib0070.c contains a similar assumption that 
> BANDWIDTH_TO_KHZ() could result in values other than {6,7,8}000
> (I haven't checked whether there are more such assumptions in other 
> places).
> 
> Spotted by the Coverity checker.
> 
> 
> cu
> Adrian
> 
> -- 
> 
>        "Is there not promise of rain?" Ling Tan asked suddenly out
>         of the darkness. There had been need of rain for many days.
>        "Only a promise," Lao Er said.
>                                        Pearl S. Buck - Dragon Seed
> 
> 
> _______________________________________________
> v4l-dvb-maintainer mailing list
> v4l-dvb-maintainer@linuxtv.org
> http://www.linuxtv.org/cgi-bin/mailman/listinfo/v4l-dvb-maintainer
> 

Re: [v4l-dvb-maintainer] DVB: BANDWIDTH_TO_KHZ strangeness

[Mauro Carvalho Chehab]

> I know that in the future the linux-dvb-API will also support other 
> bandwidths so I'm begging for mercy for those 3 things here to not get too 
> much out-of-sync with our internal code.

I don't see much problem on keeping this for a while. 

However, if not causing to much troubles for you to manage, I would to
this, instead:
#if 0
	/* Currently, DVB API allows only bandwidths starting from 5 GHz */
        factor = BANDWIDTH_TO_KHZ(ch->u.ofdm.bandwidth);
        if (factor >= 5000)
                factor = 1;
        else
                factor = 6;
#else
	factor = 6;
#endif

With the above code, gentree.pl scripts will automatically remove the
dead code from the Kernel, while keeping it defined at the development
environment.

If you want, you may also replace the #if 0 by something like:

#ifdef API_SUPPORTS_LOW_BANDWIDTH

In this case, by adding API_SUPPORTS_LOW_BANDWIDTH to gentree.pl, the
same effect of eliminating the dead code from kernel can be produced,
since gentree.pl is capable of evaluating cpp macros like the above to 0
(or 1) for the constants declared on a table inside its code.

-- 
Cheers,
Mauro

Re: [v4l-dvb-maintainer] DVB: BANDWIDTH_TO_KHZ strangeness

[Adrian Bunk]

On Mon, Oct 22, 2007 at 04:29:36PM -0200, Mauro Carvalho Chehab wrote:
> 
> > I know that in the future the linux-dvb-API will also support other 
> > bandwidths so I'm begging for mercy for those 3 things here to not get too 
> > much out-of-sync with our internal code.
> 
> I don't see much problem on keeping this for a while. 
> 
> However, if not causing to much troubles for you to manage, I would to
> this, instead:
> #if 0
> 	/* Currently, DVB API allows only bandwidths starting from 5 GHz */
>         factor = BANDWIDTH_TO_KHZ(ch->u.ofdm.bandwidth);
>         if (factor >= 5000)
>                 factor = 1;
>         else
>                 factor = 6;
> #else
> 	factor = 6;
> #endif
> 
> With the above code, gentree.pl scripts will automatically remove the
> dead code from the Kernel, while keeping it defined at the development
> environment.
>...

Good compilers like gcc generate the same code for both cases [1], so 
there's no reason for changing anything.

The reason for my email was that it looked strange, but since it's 
intended I'd say it's OK.

> Cheers,
> Mauro

cu
Adrian

[1] except that your #else case contains the wrong value  ;-)

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [v4l-dvb-maintainer] DVB: BANDWIDTH_TO_KHZ strangeness

[Mauro Carvalho Chehab]

Em Seg, 2007-10-22 às 21:54 +0200, Adrian Bunk escreveu:
> [1] except that your #else case contains the wrong value  ;-)

This is what happens when you try to write "quick" fixes ;)

/me is needing a day with 48 hours...

-- 
Cheers,
Mauro