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
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
> [email protected]
> http://www.linuxtv.org/cgi-bin/mailman/listinfo/v4l-dvb-maintainer
>
> 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
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
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