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