Suspicious code in revision 867140 (Left Items)
bartoschek at gmx.de
Mon Oct 6 07:48:18 BST 2008
I've removed all items that got feedback to make the list shorter.
The following items are for revision 864329:
If the condition in line 225 is true for the first iteration then the shift
amount is i - 1 == -1 in line 226. This is invalid.
Line 604 indicates that mVerticalLinesDistance can be 0. If this is the case
and the execution reaches this line, then a devision by 0 is the result.
- kdebase/workspace/ksysguard/gui/WorkSheet.cc:548, 551
If the condition in line 538 is false then newDisplay is NULL here.
There is no need to confuse the reader and use the bitwise-or here.
If line 270 is false and line 278 is false but line 298 is true, then butly
is NULL here.
The following items are for revision 867140:
It seems as if i and columns are not changed in the loop. An endless loop is
Line 234 indicates that declarator can be NULL. A crash follows here
high.red is quint16 and therefore always >= 0.
- kdesupport/qimageblitz/blitz/scale.cpp:165, 192, 223
If dh (dw, d) is 0 then a division by 0 is performed.
Can mimeTypeFactory or serviceFactory or servicetypeFactory be still NULL
after the loop in line 490?
The result of the dynamic_casts is not used.
Line 395 indicates that myMenu can be NULL here.
A default case for the switch in line 239 would catch possible errors if
additional states are added.
A default case for the switch in line 186 would catch possible errors if
additional modes are added.
!rotation can only be 0 or 1. m_rotations is an int. I guess
"if (not (rotation & m_rotations))" is correct here.
- kdebase/workspace/kcontrol/randr/randrcrtc.cpp:269, 277, 290, 295
Memory leak. outputs is not deleted. This is a strong hint that one should
No need to use boolean arithmetic.
It is not idiomatic for C++ to include the last element into the range. I
that the code is correct but the Phonon::Category enum is badly defined.
breaking unconditionally from the loop? If the code is correct than I would
suggest the usage of an if-condition instead.
- kdebase/apps/konqueror/src/konqbookmarkbar.cpp:250, 264,285
No need to use boolean arithmetic
A break might be missing here. At least a comment.
The compiler reads here
bool hasSwitch = (hasCommonSwitch | capture)
? snd_mixer_selem_has_capture_switch ( elem )
: snd_mixer_selem_has_playback_switch ( elem );
I think this is not intended.
If the default case is selected then statusValue is uninitialized
A break is missing.
A break might be missing.
This loop does not iterate.
Line 277 indicates that me can be NULL here.
Line 551 indicates that cleartextData might be NULL here.
Line 391 indicates that cryptProto might be NULL here.
Line 742 indicates that m_tool can be NULL here.
Line 185 indicates that mWidget->lvAddresses can be NULL here.
Line 1409 indicates that r might be NULL here.
Line 258 indicates that item might be NULL here.
Line 171 indicates that rec might be NULL here.
memo is NULL here if line 265 is never executed.
- kdepim/akregator/src/subscriptionlistmodel.cpp:151, 159
Line 855 indicates that item might be NULL here.
If line 84 is true, then result is NULL here.
Boolean arithmetic here?
Line 211 indicates that ret can be NULL here.
context->type() has type KDevelop::DUContext::ContextType. Global however has
Ensuring that m_noSystemEnvironmentPath really has the value false?
- kdevelop/buildtools/managers/cmake/parser/cmakeast.cpp:3055, 3486
Is a break missing?
Line 136 indicates that a default case is possible and type is undefined
here. Maybe an assertion should be added to the default case.
A break is missing.
val is not initialized if line 574 is never executed. Maybe ok should be
My checker says that the condition is always false and I think this code is
broken, but I cannot say why. t is a pointer-class and there is boolean
arithmetic with an enum type. My hypothesis is that t is converted to bool
using AbstractType::operator bool() and is used for the condition. However
a bool can only be 0 or 1 and therefore the condition is always false because
VolatileModifier is 2.
To prevent such conversions there should NEVER be an operator bool(). There
is a better idiom, but I do not know how it is called (comes from boost)
typedef T * UPtr::* UnspecifiedBoolType;
operator UnspecifiedBoolType() const
return _d == NULL ? NULL : &TypePtr<T>::d;
Line 89 indicates that baseConversionLevels can be NULL here.
A break might be missing.
More information about the kde-core-devel