Results of the khtml scanning

Thiago Macieira thiago at kde.org
Sun Apr 2 10:48:05 BST 2006


Ismail Donmez wrote:
>Remove that line because root is not used
>
>- dcop/dcoptypes.h:77-86
>
>The last dcopTypeInit function does not change the argument as the
> others. It could be written as: inline void dcopTypeInit(const char*){}
>But is this intended?

No. The last one could be written:
template <class T> inline void dcopTypeInit(T& t){ t = T(); }

But I wouldn't change this now, since it may break existing applications.

>- khtml/rendering/render_block.cpp:2481
>
>The result of RenderFlow::nodeAtPoint is bool but you use it in a
> bitwise operation, althoug inBox is also a bool.  Why not  ||=?

Because the operator ||= doesn't exist:
/tmp/testcase2.cpp:12: error: expected primary-expression before '=' token

Besides, unlike ||, the | operator evaluates both sides. This may be the 
whole reason why it is there. If it were ||=, then 
RenderFlow::nodeAtPoint would never be called if inBox were already true. 
However, if the side effect isn't necessary, then there is one 
unnecessary call.

>- khtml/rendering/render_block.cpp:2476
>
>The same as the above mistake. Use ||=

Same. This is actually even less likely to be the case, since it's inside 
a for-loop.

>- khtml/rendering/render_table.cpp:2174
>
>Use ||= instead of |=

Same.

>
>- khtml/rendering/render_text.cpp:400 - 437
>
>Why such a complicated  then part of the if condition?

To be able to "break". That's ugly.

>- khtml/rendering/render_object.cpp:1678
>
>Use ||= instead of |=

Same comment as before about side-effects.

>
>- khtml/rendering/render_form.cpp:1152,1157
>
>Use ||= instead of |=

This is a true case of where ||= would be useful.

Too bad it doesn't exist.

>- khtml/rendering/render_image.cpp:427
>
>Use ||= instead of |=

Side-effects.

>- khtml/khtmlview.cpp:893
>
>Use ||= instead of |=.

I can't find this. The only |= in khtmlview.cpp is on a QRect.

>- khtml/rendering/render_inline.cpp:867, 803, 790, 545, 2127
>
>Boolean variables are used in arithmetic expressions.

Line numbers are off:
     * @return actual begin of point array   <-- line 545
    {    <--- line 803

The file has 952 lines. There can be no line 2127.

I have no idea what is meant here.

>- khtml/khtml_part.cpp:6324
>
>Use != instead of ^.

Yes, unless XOR is faster.

>- khtml/css/cssparser.cpp:644, 674, 736, 744, 748, 756, 768, 776, 796,
> 805, 820, 840, 941, 952, 958 khtml/css/cssstyleselector.cpp:2199, 2221,
> 2265, 3066, 3340, 3544
>
>Boolean variables are used in arithmetic expressions.

& and | used in booleans. Maybe it's faster?

cssparser.cpp is yacc/bison-generated, for the most part.

-- 
Thiago Macieira  -  thiago (AT) macieira.info - thiago (AT) kde.org
  thiago.macieira (AT) trolltech.com     Trolltech AS
    GPG: 0x6EF45358                   |  Sandakerveien 116,
    E067 918B B660 DBD1 105C          |  NO-0402
    966C 33F5 F005 6EF4 5358          |  Oslo, Norway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: not available
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20060402/166b91cd/attachment.sig>


More information about the kfm-devel mailing list