Some PVS-Studio and cppcheck fixes

cppcheck:
[apps/esmtool/record.cpp:697]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[apps/esmtool/record.cpp:1126]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[apps/esmtool/record.cpp:1138]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[apps/niftest/niftest.cpp:36]: (performance) Function parameter 'filename' should be passed by reference.
[apps/niftest/niftest.cpp:41]: (performance) Function parameter 'filename' should be passed by reference.
[apps/opencs/model/prefs/boolsetting.cpp:25]: (warning) Possible leak in public function. The pointer 'mWidget' is not deallocated before it is allocated.
[apps/opencs/model/prefs/shortcuteventhandler.cpp:52]: (warning) Return value of std::remove() ignored. Elements remain in container.
[apps/openmw/mwstate/quicksavemanager.cpp:5]: (performance) Variable 'mSaveName' is assigned in constructor body. Consider performing initialization in initialization list.

PVS-Studio:
apps/opencs/model/filter/parser.cpp  582  warn  V560 A part of conditional expression is always true: allowPredefined.
apps/opencs/view/world/referencecreator.cpp  67  warn  V547 Expression '!errors.empty()' is always false.
apps/opencs/view/world/referencecreator.cpp  74  warn  V547 Expression '!errors.empty()' is always false.
apps/opencs/view/doc/loader.cpp  170  warn  V560 A part of conditional expression is always true: !completed.
apps/opencs/view/doc/loader.cpp  170  warn  V560 A part of conditional expression is always true: !error.empty().
apps/opencs/model/tools/pathgridcheck.cpp  32  err  V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 32, 34.
apps/opencs/model/world/refidadapterimp.cpp  1376  err  V547 Expression 'subColIndex < 3' is always true.

apps/openmw/mwgui/widgets.hpp  318  warn  V703 It is odd that the 'mEnableRepeat' field in derived class 'MWScrollBar' overwrites field in base class 'ScrollBar'. Check lines: widgets.hpp:318, MyGUI_ScrollBar.h:179.
apps/openmw/mwgui/widgets.hpp  319  warn  V703 It is odd that the 'mRepeatTriggerTime' field in derived class 'MWScrollBar' overwrites field in base class 'ScrollBar'. Check lines: widgets.hpp:319, MyGUI_ScrollBar.h:180.
apps/openmw/mwgui/widgets.hpp  320  warn  V703 It is odd that the 'mRepeatStepTime' field in derived class 'MWScrollBar' overwrites field in base class 'ScrollBar'. Check lines: widgets.hpp:320, MyGUI_ScrollBar.h:181
apps/openmw/mwmechanics/actors.cpp  1425  warn  V547 Expression '!detected' is always true.
apps/openmw/mwmechanics/character.cpp  2155  err  V547 Expression 'mode == 0' is always true.
apps/openmw/mwmechanics/character.cpp  1192  warn  V592 The expression was enclosed by parentheses twice: ((expression)). One pair of parentheses is unnecessary or misprint is present.
apps/openmw/mwmechanics/character.cpp  521  warn  V560 A part of conditional expression is always true: (idle == mIdleState).
apps/openmw/mwmechanics/pathfinding.cpp  317  err  V547 Expression 'mPath.size() >= 2' is always true.
apps/openmw/mwscript/interpretercontext.cpp  409  warn  V560 A part of conditional expression is always false: rank > 9.
apps/openmw/mwgui/windowbase.cpp  28  warn  V560 A part of conditional expression is always true: !visible.
apps/openmw/mwgui/journalwindow.cpp  561  warn  V547 Expression '!mAllQuests' is always false.
apps/openmw/mwgui/referenceinterface.cpp  18  warn  V571 Recurring check. The '!mPtr.isEmpty()' condition was already verified in line 16.
apps/openmw/mwworld/scene.cpp  463  warn  V547 Expression 'adjustPlayerPos' is always true.
apps/openmw/mwworld/worldimp.cpp  409  err  V766 An item with the same key '"sCompanionShare"' has already been added.
apps/openmw/mwworld/cellstore.cpp  691  warn  V519 The 'state.mWaterLevel' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 689, 691.
apps/openmw/mwworld/weather.cpp  1125  warn  V519 The 'mResult.mParticleEffect' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1123, 1125.
apps/openmw/mwworld/weather.cpp  1137  warn  V519 The 'mResult.mParticleEffect' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1135, 1137.

apps/wizard/unshield/unshieldworker.cpp  475  warn  V728 An excessive check can be simplified. The '(A && B) || (!A && !B)' expression is equivalent to the 'bool(A) == bool(B)' expression.
apps/wizard/installationpage.cpp  163  warn  V735 Possibly an incorrect HTML. The "</p" closing tag was encountered, while the "</span" tag was expected.

components/fontloader/fontloader.cpp  427  err  V547 Expression 'i == 1' is always true.
components/nifosg/nifloader.cpp  282  warn  V519 The 'created' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 278, 282.
components/esm/loadregn.cpp  119  err  V586 The 'clear' function is called twice for deallocation of the same resource. Check lines: 112, 119.
components/esm/cellref.cpp  178  warn  V581 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 175, 178.
components/esmterrain/storage.cpp  235  warn  V560 A part of conditional expression is always true: colStart == 0.
components/esmterrain/storage.cpp  237  warn  V560 A part of conditional expression is always true: rowStart == 0.

apps/esmtool/record.cpp

@@ -694,7 +694,7 @@ void Record<ESM::Dialogue>::print()
     // loads, rather than loading and then dumping. :-( Anyone mind if
     // I change this?
     ESM::Dialogue::InfoContainer::iterator iit;
-    for (iit = mData.mInfo.begin(); iit != mData.mInfo.end(); iit++)
+    for (iit = mData.mInfo.begin(); iit != mData.mInfo.end(); ++iit)
         std::cout << "INFO!" << iit->mId << std::endl;
 }
 
@@ -1123,7 +1123,7 @@ void Record<ESM::Pathgrid>::print()
 
     int i = 0;
     ESM::Pathgrid::PointList::iterator pit;
-    for (pit = mData.mPoints.begin(); pit != mData.mPoints.end(); pit++)
+    for (pit = mData.mPoints.begin(); pit != mData.mPoints.end(); ++pit)
     {
         std::cout << "  Point[" << i << "]:" << std::endl;
         std::cout << "    Coordinates: (" << pit->mX << ","
@@ -1135,7 +1135,7 @@ void Record<ESM::Pathgrid>::print()
     }
     i = 0;
     ESM::Pathgrid::EdgeList::iterator eit;
-    for (eit = mData.mEdges.begin(); eit != mData.mEdges.end(); eit++)
+    for (eit = mData.mEdges.begin(); eit != mData.mEdges.end(); ++eit)
     {
         std::cout << "  Edge[" << i << "]: " << eit->mV0 << " -> " << eit->mV1 << std::endl;
         if (eit->mV0 >= mData.mData.mS2 || eit->mV1 >= mData.mData.mS2)

apps/niftest/niftest.cpp

@@ -33,12 +33,12 @@ bool hasExtension(std::string filename, std::string  extensionToFind)
 }
 
 ///See if the file has the "nif" extension.
-bool isNIF(std::string filename)
+bool isNIF(const std::string & filename)
 {
     return hasExtension(filename,"nif");
 }
 ///See if the file has the "bsa" extension.
-bool isBSA(std::string filename)
+bool isBSA(const std::string & filename)
 {
     return hasExtension(filename,"bsa");
 }

apps/opencs/model/filter/parser.cpp

@@ -579,7 +579,7 @@ bool CSMFilter::Parser::parse (const std::string& filter, bool allowPredefined)
     }
     // We do not use isString() here, because there could be a pre-defined filter with an ID that is
     // equal a filter keyword.
-    else if (token.mType==Token::Type_String && allowPredefined)
+    else if (token.mType==Token::Type_String)
     {
         if (getNextToken()!=Token (Token::Type_EOS))
         {

apps/opencs/model/prefs/boolsetting.cpp

@@ -22,6 +22,8 @@ CSMPrefs::BoolSetting& CSMPrefs::BoolSetting::setTooltip (const std::string& too
 
 std::pair<QWidget *, QWidget *> CSMPrefs::BoolSetting::makeWidgets (QWidget *parent)
 {
+    if (mWidget != nullptr)
+        delete mWidget;
     mWidget = new QCheckBox (QString::fromUtf8 (getLabel().c_str()), parent);
     mWidget->setCheckState (mDefault ? Qt::Checked : Qt::Unchecked);
 

apps/opencs/model/prefs/shortcuteventhandler.cpp

@@ -49,7 +49,7 @@ namespace CSMPrefs
         ShortcutMap::iterator shortcutListIt = mWidgetShortcuts.find(widget);
         if (shortcutListIt != mWidgetShortcuts.end())
         {
-            std::remove(shortcutListIt->second.begin(), shortcutListIt->second.end(), shortcut);
+            shortcutListIt->second.erase(std::remove(shortcutListIt->second.begin(), shortcutListIt->second.end(), shortcut), shortcutListIt->second.end());
         }
     }
 

apps/opencs/model/tools/pathgridcheck.cpp

@@ -29,7 +29,7 @@ void CSMTools::PathgridCheckStage::perform (int stage, CSMDoc::Messages& message
     CSMWorld::UniversalId id (CSMWorld::UniversalId::Type_Pathgrid, pathgrid.mId);
 
     // check the number of pathgrid points
-    if (pathgrid.mData.mS2 > static_cast<int>(pathgrid.mPoints.size()))
+    if (pathgrid.mData.mS2 < static_cast<int>(pathgrid.mPoints.size()))
         messages.add (id, pathgrid.mId + " has less points than expected", "", CSMDoc::Message::Severity_Error);
     else if (pathgrid.mData.mS2 > static_cast<int>(pathgrid.mPoints.size()))
         messages.add (id, pathgrid.mId + " has more points than expected", "", CSMDoc::Message::Severity_Error);

apps/opencs/model/world/refcollection.cpp

@@ -24,7 +24,7 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool
     bool isDeleted = false;
 
     // hack to initialise mindex
-    while (!(mref.mRefNum.mIndex = 0) && ESM::Cell::getNextRef(reader, ref, isDeleted, true, &mref))
+    while (!(mref.mRefNum.mIndex == 0) && ESM::Cell::getNextRef(reader, ref, isDeleted, true, &mref))
     {
         // Keep mOriginalCell empty when in modified (as an indicator that the
         // original cell will always be equal the current cell).

apps/opencs/model/world/refidadapterimp.cpp

@@ -1373,10 +1373,8 @@ QVariant CSMWorld::CreatureAttackRefIdAdapter::getNestedData (const RefIdColumn
 
     if (subColIndex == 0)
         return subRowIndex + 1;
-    else if (subColIndex < 3) // 1 or 2
+    else // 1 or 2
         return creature.mData.mAttack[(subRowIndex * 2) + (subColIndex - 1)];
-    else
-        return QVariant(); // throw an exception here?
 }
 
 void CSMWorld::CreatureAttackRefIdAdapter::setNestedData (const RefIdColumn *column,

apps/opencs/view/doc/loader.cpp

@@ -167,7 +167,7 @@ void CSVDoc::Loader::loadingStopped (CSMDoc::Document *document, bool completed,
         delete iter->second;
         mDocuments.erase (iter);
     }
-    else if (!completed && !error.empty())
+    else
     {
         iter->second->abort (error);
         // Leave the window open for now (wait for the user to close it)

apps/opencs/view/world/referencecreator.cpp

@@ -63,19 +63,9 @@ std::string CSVWorld::ReferenceCreator::getErrors() const
     std::string cell = mCell->text().toUtf8().constData();
 
     if (cell.empty())
-    {
-        if (!errors.empty())
-            errors += "<br>";
-
         errors += "Missing Cell ID";
-    }
     else if (getData().getCells().searchId (cell)==-1)
-    {
-        if (!errors.empty())
-            errors += "<br>";
-
         errors += "Invalid Cell ID";
-    }
 
     return errors;
 }

apps/openmw/mwgui/journalwindow.cpp

@@ -558,7 +558,7 @@ namespace
             if (mAllQuests)
             {
                 SetNamesInactive setInactive(list);
-                mModel->visitQuestNames(!mAllQuests, setInactive);
+                mModel->visitQuestNames(false, setInactive);
             }
 
             MWBase::Environment::get().getWindowManager()->playSound("book page");

apps/openmw/mwgui/referenceinterface.cpp

@@ -14,12 +14,9 @@ namespace MWGui
     {
         // check if count of the reference has become 0
         if (!mPtr.isEmpty() && mPtr.getRefData().getCount() == 0)
-        {
-            if (!mPtr.isEmpty())
         {
             mPtr = MWWorld::Ptr();
             onReferenceUnavailable();
         }
     }
 }
-}

apps/openmw/mwgui/widgets.cpp

@@ -532,12 +532,11 @@ namespace MWGui
             assignWidget(mBarTextWidget, "BarText");
         }
 
-        MWScrollBar::MWScrollBar()
-            : mEnableRepeat(true)
-            , mRepeatTriggerTime(0.5f)
-            , mRepeatStepTime(0.1f)
-            , mIsIncreasing(true)
+        MWScrollBar::MWScrollBar() : mIsIncreasing(true)
         {
+            mEnableRepeat = true;
+            mRepeatTriggerTime = 0.5f;
+            mRepeatStepTime = 0.1f;
 #if MYGUI_VERSION >= MYGUI_DEFINE_VERSION(3,2,2)
             ScrollBar::setRepeatEnabled(false);
 #endif

apps/openmw/mwgui/widgets.hpp

@@ -315,9 +315,6 @@ namespace MWGui
             virtual void initialiseOverride();
             void repeatClick(MyGUI::Widget* _widget, MyGUI::ControllerItem* _controller);
 
-            bool mEnableRepeat;
-            float mRepeatTriggerTime;
-            float mRepeatStepTime;
             bool mIsIncreasing;
 
         private:

apps/openmw/mwgui/windowbase.cpp

@@ -25,7 +25,7 @@ void WindowBase::setVisible(bool visible)
 
     if (visible)
         onOpen();
-    else if (wasVisible && !visible)
+    else if (wasVisible)
         onClose();
 
     // This is needed as invisible widgets can retain key focus.

apps/openmw/mwmechanics/actors.cpp

@@ -1422,7 +1422,7 @@ namespace MWMechanics
                                 MWBase::Environment::get().getWindowManager()->setSneakVisibility(false);
                                 break;
                             }
-                            else if (!detected)
+                            else
                                 avoidedNotice = true;
                         }
                     }

apps/openmw/mwmechanics/character.cpp

@@ -517,8 +517,7 @@ void CharacterController::refreshMovementAnims(const WeaponInfo* weap, Character
 
 void CharacterController::refreshIdleAnims(const WeaponInfo* weap, CharacterState idle, bool force)
 {
-    if(force || idle != mIdleState ||
-        ((idle == mIdleState) && !mAnimation->isPlaying(mCurrentIdle) && mAnimQueue.empty()))
+    if(force || idle != mIdleState || (!mAnimation->isPlaying(mCurrentIdle) && mAnimQueue.empty()))
     {
         mIdleState = idle;
         size_t numLoops = ~0ul;
@@ -1189,7 +1188,7 @@ bool CharacterController::updateWeaponState()
         std::string weapgroup;
         if(weaptype == WeapType_None)
         {
-            if ((!isWerewolf || mWeaponType != WeapType_Spell))
+            if (!isWerewolf || mWeaponType != WeapType_Spell)
             {
                 getWeaponGroup(mWeaponType, weapgroup);
                 mAnimation->play(weapgroup, priorityWeapon,
@@ -2152,7 +2151,7 @@ bool CharacterController::playGroup(const std::string &groupname, int mode, int
                             MWRender::Animation::BlendMask_All, false, 1.0f,
                             ((mode==2) ? "loop start" : "start"), "stop", 0.0f, count-1, loopfallback);
     }
-    else if(mode == 0)
+    else
     {
         mAnimQueue.resize(1);
         mAnimQueue.push_back(entry);

apps/openmw/mwmechanics/pathfinding.cpp

@@ -314,8 +314,6 @@ namespace MWMechanics
         {
             const ESM::Pathgrid::Point oldStart(*getPath().begin());
             buildPath(startPoint, endPoint, cell, pathgridGraph);
-            if (mPath.size() >= 2)
-            {
             // if 2nd waypoint of new path == 1st waypoint of old, 
             // delete 1st waypoint of new path.
             std::list<ESM::Pathgrid::Point>::iterator iter = ++mPath.begin();
@@ -327,7 +325,6 @@ namespace MWMechanics
             }
         }
     }
-    }
 
     const MWWorld::CellStore* PathFinder::getPathCell() const
     {

apps/openmw/mwscript/interpretercontext.cpp

@@ -406,7 +406,7 @@ namespace MWScript
         const MWWorld::ESMStore &store = world->getStore();
         const ESM::Faction *faction = store.get<ESM::Faction>().find(factionId);
 
-        if(rank < 0 || rank > 9)
+        if(rank < 0)
             return "";
 
         return faction->mRanks[rank];

apps/openmw/mwstate/quicksavemanager.cpp

@@ -1,8 +1,8 @@
 #include "quicksavemanager.hpp"
 
 MWState::QuickSaveManager::QuickSaveManager(std::string &saveName, unsigned int maxSaves)
+  : mSaveName(saveName)
 {
-    this->mSaveName = saveName;
     this->mMaxSaves = maxSaves;
     this->mOldestSlotVisited = NULL;
     this->mSlotsVisited = 0;

apps/openmw/mwworld/cellstore.cpp

@@ -685,8 +685,8 @@ namespace MWWorld
     {
         state.mId = mCell->getCellId();
 
-        if (mCell->mData.mFlags & ESM::Cell::Interior && mCell->mData.mFlags & ESM::Cell::HasWater)
-            state.mWaterLevel = mWaterLevel;
+        // if (mCell->mData.mFlags & ESM::Cell::Interior && mCell->mData.mFlags & ESM::Cell::HasWater)
+        //     state.mWaterLevel = mWaterLevel;
 
         state.mWaterLevel = mWaterLevel;
         state.mHasFogOfWar = (mFogState.get() ? 1 : 0);

apps/openmw/mwworld/scene.cpp

@@ -460,7 +460,6 @@ namespace MWWorld
             float z = pos.rot[2];
             world->rotateObject(player, x, y, z);
 
-            if (adjustPlayerPos)
             player.getClass().adjustPosition(player, true);
         }
 

apps/openmw/mwworld/weather.cpp

@@ -1134,7 +1134,6 @@ inline void WeatherManager::calculateTransitionResult(const float factor, const
         mResult.mIsStorm = current.mIsStorm;
         mResult.mParticleEffect = current.mParticleEffect;
         mResult.mRainEffect = current.mRainEffect;
-        mResult.mParticleEffect = current.mParticleEffect;
         mResult.mRainSpeed = current.mRainSpeed;
         mResult.mRainFrequency = current.mRainFrequency;
         mResult.mAmbientSoundVolume = 1-(factor*2);
@@ -1146,7 +1145,6 @@ inline void WeatherManager::calculateTransitionResult(const float factor, const
         mResult.mIsStorm = other.mIsStorm;
         mResult.mParticleEffect = other.mParticleEffect;
         mResult.mRainEffect = other.mRainEffect;
-        mResult.mParticleEffect = other.mParticleEffect;
         mResult.mRainSpeed = other.mRainSpeed;
         mResult.mRainFrequency = other.mRainFrequency;
         mResult.mAmbientSoundVolume = 2*(factor-0.5f);

apps/openmw/mwworld/worldimp.cpp

@@ -406,7 +406,6 @@ namespace MWWorld
         gmst["sCompanionWarningMessage"] = ESM::Variant("Warning message");
         gmst["sCompanionWarningButtonOne"] = ESM::Variant("Button 1");
         gmst["sCompanionWarningButtonTwo"] = ESM::Variant("Button 2");
-        gmst["sCompanionShare"] = ESM::Variant("Companion Share");
         gmst["sProfitValue"] = ESM::Variant("Profit Value");
         gmst["sTeleportDisabled"] = ESM::Variant("Teleport disabled");
         gmst["sLevitateDisabled"] = ESM::Variant("Levitate disabled");

apps/wizard/installationpage.cpp

@@ -161,7 +161,7 @@ void Wizard::InstallationPage::showFileDialog(Wizard::Component component)
 
     if (path.isEmpty()) {
         logTextEdit->appendHtml(tr("<p><br/><span style=\"color:red;\"> \
-                                    <b>Error: The installation was aborted by the user</b></p>"));
+                                    <b>Error: The installation was aborted by the user</b></span></p>"));
 
         mWizard->addLogText(QLatin1String("Error: The installation was aborted by the user"));
         mWizard->mError = true;

apps/wizard/unshield/unshieldworker.cpp

@@ -472,8 +472,8 @@ bool Wizard::UnshieldWorker::setupComponent(Component component)
 
                 if (morrowindFound) {
                     // Check if we have correct archive, other archives have Morrowind.bsa too
-                    if ((tribunalFound && bloodmoonFound)
-                            || (!tribunalFound && !bloodmoonFound)) {
+                    if (tribunalFound == bloodmoonFound)
+                    {
                         cabFile = file;
                         found = true; // We have a GoTY disk or a Morrowind-only disk
                     }

components/esm/cellref.cpp

@@ -173,10 +173,10 @@ void ESM::CellRef::save (ESMWriter &esm, bool wideRefNum, bool inInventory, bool
     }
 
     if (!inInventory)
+    {
         esm.writeHNOCString ("KNAM", mKey);
-
-    if (!inInventory)
         esm.writeHNOCString ("TNAM", mTrap);
+    }
 
     if (mReferenceBlocked != -1)
         esm.writeHNT("UNAM", mReferenceBlocked);

components/esm/loadregn.cpp

@@ -109,8 +109,6 @@ namespace ESM
 
     void Region::blank()
     {
-        mName.clear();
-
         mData.mClear = mData.mCloudy = mData.mFoggy = mData.mOvercast = mData.mRain =
             mData.mThunder = mData.mAsh, mData.mBlight = mData.mA = mData.mB = 0;
 

components/esmterrain/storage.cpp

@@ -232,9 +232,9 @@ namespace ESMTerrain
                 // Skip the first row / column unless we're at a chunk edge,
                 // since this row / column is already contained in a previous cell
                 // This is only relevant if we're creating a chunk spanning multiple cells
-                if (colStart == 0 && vertY_ != 0)
+                if (vertY_ != 0)
                     colStart += increment;
-                if (rowStart == 0 && vertX_ != 0)
+                if (vertX_ != 0)
                     rowStart += increment;
 
                 // Only relevant for chunks smaller than (contained in) one cell

components/fontloader/fontloader.cpp

@@ -448,7 +448,7 @@ namespace Gui
             MyGUI::FontCodeType::Enum type;
             if(i == 0)
                 type = MyGUI::FontCodeType::Selected;
-            else if (i == 1)
+            else
                 type = MyGUI::FontCodeType::SelectedBack;
 
             MyGUI::xml::ElementPtr cursorCode = codes->createChild("Code");

components/nifosg/nifloader.cpp

@@ -275,7 +275,6 @@ namespace NifOsg
                     for (unsigned int i=0; i<root->getNumChildren(); ++i)
                         skel->addChild(root->getChild(i));
                     root->removeChildren(0, root->getNumChildren());
-                    created = skel;
                 }
                 else
                     skel->addChild(created);