[bartoschek at gmx.de: Suspicious code in revision 867140 (Part 2)]

[Andreas Pakulat]

Hi,

Cristoph posted the following list of suspicious code to kde-core-devel and
as not all of us might follow that list I'm forwarding the relevant parts:

If somebody fixes any of the issues or finds that its a "false positive"
please reply to this list, I'll be forwarding the answers in
kde-core-devel.


----- Forwarded message from Christoph Bartoschek <bartoschek at gmx.de> -----

From: Christoph Bartoschek <bartoschek at gmx.de>
To: kde-core-devel at kde.org
Subject: Suspicious code in revision 867140 (Part 2)
Reply-To: kde-core-devel at kde.org
List-Id: KDE Core Development <kde-core-devel.kde.org>
List-Unsubscribe: <https://mail.kde.org/mailman/listinfo/kde-core-devel>,
	<mailto:kde-core-devel-request at kde.org?subject=unsubscribe>
List-Post: <mailto:kde-core-devel at kde.org>
List-Help: <mailto:kde-core-devel-request at kde.org?subject=help>
List-Subscribe: <https://mail.kde.org/mailman/listinfo/kde-core-devel>,
	<mailto:kde-core-devel-request at kde.org?subject=subscribe>

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


----- End forwarded message -----

-- 
You prefer the company of the opposite sex, but are well liked by your own.