Suspicious code in revision 867140 (Part 2)

[Robert Knight]

Hi Christoph,

Sorry if this has already been answered but I couldn't find the answer
elsewhere.  What is the tool you're using to do this analysis and is
it available for others to use?

Regards,
Robert.

2008/10/4 Christoph Bartoschek <bartoschek at gmx.de>:
> - 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.
>
>