brektrou / rtl8821CU

Realtek RTL8811CU/RTL8821CU USB Wi-Fi adapter driver for Linux
GNU General Public License v2.0
1.59k stars 461 forks source link

ceil() macro in core/rtw_mp.c is mathematically incorrect #92

Open pprindeville opened 3 years ago

pprindeville commented 3 years ago

I'm looking at this sequence:

#define CEILING_POS(X) ((X - (int)(X)) > 0 ? (int)(X + 1) : (int)(X))
#define CEILING_NEG(X) ((X - (int)(X)) < 0 ? (int)(X - 1) : (int)(X))
#define ceil(X) (((X) > 0) ? CEILING_POS(X) : CEILING_NEG(X))

and the CEILING_NEG() function stands out as being incorrect. The correct definition of ceil() as defined by:

the least integer greater than or equal to 'x'

would require this definition instead:

#define CEILING_NEG(X) ((int)(X))

i.e. for any x < 0, the ceiling would therefore be x - fract(x).

That is, the least integer greater than or equal to -5.1 is -5.

More broadly, why not have:

#Include <math.h>

#define ceil(X) ((int)(ceilf(X)))
pprindeville commented 3 years ago

I've not looked at where this is used to see what the impact of the inaccuracy would be.

champignoom commented 3 years ago

Being part of driver code, I think the variable is tolerable to have either an imperfect name or a slightly non-standard semantics since it's used only here, and the standard library is certainly too much of a luxury.