Suspicious code in revision 867140 (Part 2)

Christoph Bartoschek bartoschek at gmx.de
Sat Oct 4 11:44:10 BST 2008 

- 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/plasma/dataengines/nowplaying/playerinterface/mpris/mpris.cpp:344

The expression is always false. !caps can be 0 or 1. CAN_PROVIDE_METADATA
is (1 << 5)

- kdebase/workspace/plasma/shells/desktop/panelview.cpp:82

Line 74 indicates that panel can be NULL here.

- kdebase/workspace/plasma/tools/engineexplorer/main.cpp:59, 66

No need to use boolean arithmetic here.

- kdebase/workspace/plasma/tools/engineexplorer/serviceviewer.cpp:101

Line 89 indicates that m_operations can be NULL here.

- 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.

- kdebase/workspace/kwin/scene_xrender.cpp:528,530
  kdebase/workspace/kwin/scene_opengl.cpp1318,1320

If (mask & PAINT_WINDOW_OPAQUE) is true then line 526 returns false. 
Therefore
(mask & PAINT_WINDOW_OPAQUE) is 0 in line 528 and this line is always false. 
The same holds for the other cases.

- kdebase/workspace/plasma/dataengines/network/networkengine.cpp:200

Line 186 indicates that ap can be NULL here.

- 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.

- kdemultimedia/kscd/gui/kscdwindow.cpp:112

m_miniB is deleted twice. See 110.

- kdesdk/umbrello/umbrello/dialogs/parmpropdlg.cpp:135

Line 119 indicates that attr can be NULL here.

- kdesdk/umbrello/umbrello/dialogs/umloperationdialog.cpp:469

Line 448 indicates that classfier can be NULL here.

- kdesdk/umbrello/umbrello/codegenerators/textblock.cpp:192

One should use pointers instead of a reference here. A function can always 
rely 
on the fact that references are not NULL. If this is not the case a pointer
is the correct data type.

- kdesdk/umbrello/umbrello/codegenerators/tcl/tclwriter.cpp:603,310

The QTextStream from line 315 is destructed here. However mStream still holds
a pointer to it.

- kdesdk/umbrello/umbrello/associationwidget.cpp:3301 

A break is missing.

- kdesdk/umbrello/umbrello/foreignkeyconstraint.cpp:146
  kdesdk/umbrello/umbrello/uniqueconstraint.cpp:203,227

Line 144 indicates that owningParent is NULL here.

- kdesdk/umbrello/umbrello/linepath.cpp:603

Is a break missing in line 602?

- kdenetwork/kget/core/download.cpp:67

A break might be missing. At least a comment.

- kdenetwork/kget/extensions/konqueror/kget_plug_in.cpp:219

Line 162 indicates that htmlPart can be NULL here.

- kdenetwork/kget/plasma/applet/piechart/kgetpiechart.cpp:115

Useless || true here.

- 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/filesharing/advanced/propsdlgplugin/propertiespage.cpp:475

Line 472 indicates that m_sambaShare is NULL here.

- 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.

- kdenetwork/kopete/protocols/oscar/aim/aimprotocol.cpp:209

Line 202 indicates that aimAccount can be NULL here.

- kdenetwork/kopete/kopete/kopetewindow.cpp:1192

Line 1188 indicates that self can be NULL here.

- kdenetwork/kopete/kopete/config/accounts/kopeteaccountconfig.cpp:429

Statement has no effect because ident is NULL here.

- kdepim/libkdepim/kimportdialog.cpp:424
  kdepim/libkdepim/addresseelineedit.cpp:416

A fall-through comment would be good here.

- kdepim/akonadi/tray/backup.cpp:132,113
  kdepim/akonadi/tray/restore.cpp:128,80

proc has been deleted in line 130.

- kdepim/kmail/kmcomposewin.cpp:2918

autoDelete is always true from line 2897 on.

- kdepim/kmail/actionscheduler.cpp:731

Line 723 indicates that msg can be NULL here.

- kdepim/kmail/kmfolderdir.cpp:288

A final else after line 281 would prevent uninitialized folderType here.

- kdepim/kmail/kmfoldermaildir.cpp:954

The comma-operator here?  Maybe the ,0 should be one parenthesis to the left.

- 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/kmail/partNode.cpp:332

otherState has type KMMsgSignatureState.  KMMsgEncryptionProblematic however
is of type KMMsgEncryptionState.

- kdepim/kmail/kmcommands.cpp:1925 and others

Line 1879 indicates that mDestFolder can be NULL here.

- kdepim/kmail/antispamwizard.cpp:167

Line 153 indicates that mVirusRulesPage can be NULL here.

- kdepim/kmail/recipientseditor.cpp:625

If there is no it.hasNext() then line is uninitialized. maybe line 617
should initialize with NULL.

- kdepim/knotes/knote.cpp:753

Line 742 indicates that m_tool can be NULL here.

- kdepim/kalarm/eventlistmodel.cpp:218

A break might be missing.

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

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

- kdepim/ktimetracker/karmstorage.cpp:1092

Line 1084 indicates that lock might be NULL here.

- kdepim/korganizer/printing/calprintpluginbase.cpp:1438

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

A break might be missing here.

- kdepim/korganizer/koeditorfreebusy.cpp:859

Line 855 indicates that item might be NULL here.

- kdepim/korganizer/filtereditdialog.cpp:148

Line 145 indicates that mFilters can 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/qmake/qmakemanager.cpp:141

Line 99 indicates that folderitem could be NULL here.

- 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.

More information about the kde-core-devel mailing list