Suspicious code in revision 867140 (Left Items)

Mon Oct 6 07:48:18 BST 2008

Hi,

I've removed all items that got feedback to make the list shorter.

-------------------------------------------------------------------------------

The following items are for revision 864329:

- kdelibs/kate/completion/katecompletionconfig.cpp:226
  kdelibs/kate/completion/katecompletionconfig.cpp:176

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.

- kdebase/workspace/ksysguard/gui/SensorDisplayLib/SignalPlotter.cc:635

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.

- kdebase/workspace/powerdevil/kcmodule/CapabilitiesPage.cpp:205

There is no need to confuse the reader and use the bitwise-or here.

- kdebase/workspace/powerdevil/kcmodule/CapabilitiesPage.cpp:304

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:

- kdesupport/qimageblitz/blitz/convolve.cpp:1155,991

It seems as if i and columns are not changed in the loop. An endless loop is 
the result.

- kdesupport/cpptoxml/parser/binder.cpp:237

Line 234 indicates that declarator can be NULL. A crash follows here

- kdesupport/qimageblitz/blitz/histogram.cpp:251

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.

- kdelibs/kded/kbuildsycoca.cpp:516

Can mimeTypeFactory or serviceFactory or servicetypeFactory be still NULL 
after the loop in line 490?

- kdelibs/security/kcert/kcertpart.cc:860,861,869,870

The result of the dynamic_casts is not used.

- kdebase/workspace/klipper/urlgrabber.cpp:405

Line 395 indicates that myMenu can be NULL here.

- kdebase/workspace/solid/networkmanager-0.7/manager.cpp:254

A default case for the switch in line 239 would catch possible errors if
additional states are added.

- kdebase/workspace/solid/networkmanager-0.7/wirelessnetworkinterface.cpp:205

A default case for the switch in line 186 would catch possible errors if
additional modes are added.

- kdebase/workspace/kcontrol/randr/randrcrtc.cpp:345

!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 
use
std::vector<RROutput> instead.

- kdebase/workspace/kcontrol/style/kcmstyle.cpp:380,411

No need to use boolean arithmetic.

- kdebase/runtime/phonon/kcm/devicepreference.cpp:660

It is not idiomatic for C++ to include the last element into the range. I 
guess
that the code is correct but the Phonon::Category enum is badly defined.

- kdebase/runtime/phonon/kcm/backendselection.cpp:170

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

- kdebase/workspace/libs/ksysguard/processui/ksysguardprocesslist.cpp:816

A break might be missing here. At least a comment.

- kdesvn/kdemultimedia/kmix/mixer_alsa9.cpp:392

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.

- kdenetwork/kopete/libkopete/kopeteonlinestatusmanager.cpp:152

If the default case is selected then statusValue is uninitialized

- kdenetwork/knewsticker/settingsdialog.cpp:28

A break is missing.

- kdenetwork/kopete/kopete/chatwindow/chatview.cpp:626

A break might be missing.

- kdenetwork/kopete/protocols/qq/qqchatsession.cpp:472

This loop does not iterate.

- kdenetwork/kopete/protocols/sms/services/smssendprovider.cpp:235

Assignment here?

- kdenetwork/kopete/plugins/webpresence/webpresenceplugin.cpp:287

Line 277 indicates that me can be NULL here.

- kdepim/kmail/objecttreeparser.cpp:559

Line 551 indicates that cleartextData might be NULL here.

- kdepim/kmail/objecttreeparser.cpp:491

Line 391 indicates that cryptProto might be NULL here.

- kdepim/knotes/knote.cpp:753

Line 742 indicates that m_tool can be NULL here.

- kdepim/kaddressbook/editors/imeditorwidget.cpp:219

Line 185 indicates that mWidget->lvAddresses can be NULL here.

- kdepim/ktimetracker/karmstorage.cpp:1092

Line 1409 indicates that r might be NULL here.

- kdepim/kpilot/kpilot/fileInstallWidget.cc:286

Line 258 indicates that item might be NULL here.

- kdepim/kpilot/conduits/base/hhdataproxy.cc:179

Line 171 indicates that rec might be NULL here.

- kdepim/kpilot/conduits/memofileconduit/memofile-conduit.cc:283

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.

- kdepim/kode/kxforms/guihandlerflat.cpp:89

If line 84 is true, then result is NULL here.

- kdepim/kode/kxforms/editor/editorwidget.cpp:423

Boolean arithmetic here?

- kdevplatform/language/editor/documentrangeobject.cpp:216

Line 211 indicates that ret can be NULL here.

- kdevplatform/language/duchain/dumpchain.cpp:64

context->type() has type KDevelop::DUContext::ContextType. Global however has 
type ContextType.

- kdevelop/buildtools/managers/cmake/parser/cmakeast.cpp:1554

Ensuring that m_noSystemEnvironmentPath really has the value false?

- kdevelop/buildtools/managers/cmake/parser/cmakeast.cpp:3055, 3486

Is a break missing?

- kdevelop/buildtools/managers/cmake/cmakehighlighting.cpp:139

Line 136 indicates that a default case is possible and type is undefined 
here. Maybe an assertion should be added to the default case.

- kdevelop/languages/cpp/cppduchain/expressionvisitor.cpp:769

A break is missing.

- kdevelop/languages/cpp/cppduchain/expressionvisitor.cpp:582

val is not initialized if line 574 is never executed. Maybe ok should be 
evaluated afterwards.

- kdevelop/languages/cpp/cppduchain/navigationwidget.cpp:331

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;
   }

- kdevelop/languages/cpp/cppduchain/classdeclaration.cpp:97

Line 89 indicates that baseConversionLevels can be NULL here.

- kdevelop/languages/cpp/codecompletioncontext.cpp:318,362
  kdevelop/languages/cpp/setuphelpers.cpp:80

A break might be missing.