Suspicious code in kdebase 3.5.2

[Lubos Lunak]

On Saturday 08 April 2006 10:21, Christoph Bartoschek wrote:
> ------------------------------------------------------------------
> Misc problems:
> ------------------------------------------------------------------
>
> - kwin/kcmkwin/kwinrules/main.cpp:123
>
> The loop is never entered because  bit < 1 << 31 is always false before the
> first iteration is entered. bit starts at 1 and the first check is then
> 1 < (1 << 31) <==> 1 < (-2147483648) <==> false
> Never do bitshifting on signed types :)

 How true. Fixed.

>
> - kwin/utils.cpp:313
>
> !(*dot = 0) is  always true and therefore unneccessary. Or do you mean
> !(*dot ==0)?

 No, given the strchr() above it's always (*dot == '.'). Somebody was lazy or 
wanted to be cool.

> - kdesktop/lock/lockprocess.cc:199
>
> I guess the written value does not matter here

 Right, it doesn't matter.

> , but I would still initialize tmp.
>
> - kwin/clients/redmond/redmond.cpp:164
>
> Useless use of "&& false"

 That's intended ("temporary" code).

>
> - kwin/sm.cpp:243
>
> Maybe the comment says everything, but is windowType of -2 ever possible?

 Yes, in a really obscure case. I guess this one is listed because -2 is not 
part of the enum?

> -----------------------------------------------------------------
> Lines where the operator preference between & and == leads to an error.
> There are some lines of code that look like this:
> if (variable & 0xF != 0)  ...
> The compiler reads:
> if (variable & (0xF != 0))  ...
> and not
> if ((variable & 0xF) != 0)  ...
> The result is that the compiler optimizes such code to:
> if (variable & 1) ...
> because (0xF != 0) is true and this is equivalent to 1
> -----------------------------------------------------------------
>
> - kwin/geometry.cpp:1843

 Sigh. Funny that even C# still has this & braindamage.

>
> -----------------------------------------------------------------
> Cases from switch statements that fall through in some cases but
> do not have a fall through comment as in most such cases.
> ------------------------------------------------------------------
>
> - kwin/useractions.cpp:393

 Oops.

> - khotkeys/shared/actions.cpp:155

 There is a comment.

> - kwin/clients/default/kdedefault.cpp:811

 This ugly code again was actually intentional :-/.

> - kwin/clients/b2/b2client.cpp:533
> - kwin/layers.cpp:103
> - kwin/useractions.cpp:474

 Fixed.

-- 
Lubos Lunak
KDE developer
---------------------------------------------------------------------
SuSE CR, s.r.o.  e-mail: l.lunak at suse.cz , l.lunak at kde.org
Drahobejlova 27  tel: +420 2 9654 2373
190 00 Praha 9   fax: +420 2 9654 2374
Czech Republic   http://www.suse.cz/