[linux-dvb] [patch] dvb-math - math functions

[Johannes Stezenbach]

On Thu, Apr 20, 2006, christophpfister at bluemail.ch wrote:
> Now I have my functions ready [although it isn't a patch, there are two files
> attached]. For now log2 and log10 are implemented. The maximal error for
> log10 is 0.000199406% (for log2 it's even smaller...).
> 
> Any comments are welcome, also requesting for additional functions.

It would be easier to comment your code if you wouldn't
sent it as application/octet-stream attachments.

Anyway...

The code is in no way DVB specific. IMHO it should
end up in linux/include/linux/log32.h and linux/lib/log32.c.

> #include <stdint.h>

Don't use stdint.h in the kernel. Use linux/types.h.

> const unsigned char msbtable[256] = {
...
> 	7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7};

put closing brace like so:

 	7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7
};


> const unsigned short logtable[256] = {
...
> 	0xfa2f, 0xfaeb, 0xfba6, 0xfc60, 0xfd1b, 0xfdd5, 0xfe8e, 0xff47};

same

> unsigned int intlog2(unsigned int value) {

but for functions the opening brace must be in the next line:

unsigned int intlog2(unsigned int value)
{

Hm, your code depends on the fact that value and result
are 32bit entities. IMHO it's better to use u32 to
make this explicit (although I believe ints are 32bit
on all Linux platforms).

> 	/* returns: log2(value) * 2^16
> 	   wrong result if value = 0 [ 0 instead of -inf ] */

log2(0) is undefined (NaN), not -inf. Add a WANR_ON().

> 	/* first detect the msb (count begins at 0)
> 	   for that we check in which quarter the msb is
> 	      e.g. for 0x00231f56 the four quarters are |00|, |23|, |1f| and |56|
> 	   then we determine the msb of this quarter with a table lookup
> 	      msbtable[23] = 5
> 	   then we set the quarter in the context - i.e. adding (quarter - 1) * 8
> 	      5 + (3 - 1) * 8 = 21
> 	   in binary form: 0x00231f56 = |00000000|00100011|00011111|01010110|
> 	                                            ^ msb of the quarter at position 5
>                                                     ^ msb of the value at position 21 */

Please use block comment markers like so:
	/* bla bla bla
	 * bla
	 */

> 	if (value >= (1 << 16)) /* value >= 2^16 */
> 		if (value >= (1 << 24)) /* value >= 2^24 */
> 			msb = 24 + msbtable[value >> 24];
> 		else /* 2^16 <= value < 2^24 */
> 			msb = 16 + msbtable[value >> 16];
> 	else /* value < 2^16 */
> 		if (value >= (1 << 8)) /* 2^8 <= value < 2^16 */
> 			msb = 8 + msbtable[value >> 8];
> 		else /* value < 2^8 */
> 			msb = msbtable[value];

Isn't that what ffs() does?
 
>  * @param value The value [ 0 gives a wrong result ]

 * @param value The value (must be != 0)



Johannes