Fix some cppcheck findings

include/FileClasses/Shpfile.h

@@ -47,7 +47,7 @@ class Shpfile
     };
 
 public:
-    Shpfile(SDL_RWops* rwop);
+    explicit Shpfile(SDL_RWops* rwop);
     Shpfile(const Shpfile& o) = delete;
     Shpfile(Shpfile &&) = delete;
     Shpfile& operator=(const Shpfile &) = delete;

include/FileClasses/TextManager.h

@@ -74,13 +74,13 @@ class TextManager {
         \return the localized version of unlocalizedString
     */
     const std::string& getLocalized(const std::string& unlocalizedString) const {
-        const std::string& localizedString = getLocalizedRaw(unlocalizedString);
+        const std::string& localizedStringRaw = getLocalizedRaw(unlocalizedString);
 
-        if(!localizedString.empty() && localizedString[0] == '@') {
+        if(!localizedStringRaw.empty() && localizedStringRaw[0] == '@') {
             // post-process
-            return postProcessString(localizedString);
+            return postProcessString(localizedStringRaw);
         } else {
-            return localizedString;
+            return localizedStringRaw;
         }
     }
 

include/GUI/Label.h

@@ -206,11 +206,10 @@ class Label : public Widget {
         Widget::updateTextures();
 
         if(!pTexture) {
-            int fontID = this->fontID;
             std::vector<std::string> textLines = greedyWordWrap(text,
                                                                 getSize().x,
-                                                                [fontID](const std::string& tmp) {
-                                                                    return GUIStyle::getInstance().getMinimumLabelSize(tmp, fontID).x - 4;
+                                                                [font = fontID](const std::string& tmp) {
+                                                                    return GUIStyle::getInstance().getMinimumLabelSize(tmp, font).x - 4;
                                                                 });
 
             pTexture = convertSurfaceToTexture(GUIStyle::getInstance().createLabelSurface(getSize().x,getSize().y,textLines,fontID,alignment,textcolor,textshadowcolor,backgroundcolor));

include/misc/OutputStream.h

@@ -151,7 +151,7 @@ class OutputStream
 
     class eof : public OutputStream::exception {
     public:
-        explicit eof(std::string  str) noexcept : str(std::move(str)) { };
+        explicit eof(const std::string& str) noexcept : str(str) { };
         virtual ~eof() noexcept = default;
 
         const char* what() const noexcept override { return str.c_str(); }
@@ -162,7 +162,7 @@ class OutputStream
 
     class error : public OutputStream::exception {
     public:
-        explicit error(std::string  str) noexcept : str(std::move(str)) { };
+        explicit error(const std::string& str) noexcept : str(str) { };
         virtual ~error() noexcept = default;
 
         const char* what() const noexcept override { return str.c_str(); };

include/units/AirUnit.h

@@ -41,16 +41,16 @@ class AirUnit : public UnitBase
     void checkPos() override;
     bool canPass(int xPos, int yPos) const override;
 
-    virtual FixPoint getMaxSpeed() const {
+    virtual FixPoint getMaxSpeed() const override {
         return currentMaxSpeed;
     }
 
 protected:
     virtual FixPoint getDestinationAngle() const;
 
-    virtual void navigate();
-    virtual void move();
-    virtual void turn();
+    virtual void navigate() override;
+    virtual void move() override;
+    virtual void turn() override;
 
     FixPoint currentMaxSpeed;               ///< The current maximum allowed speed
 

include/units/Carryall.h

@@ -67,7 +67,7 @@ class Carryall final : public AirUnit
     void engageTarget() override;
     void pickupTarget();
     void targeting() override;
-    virtual void turn();
+    virtual void turn() override;
 
     // unit state/properties
     std::list<Uint32>   pickedUpUnitList;   ///< What units does this carryall carry?

include/units/Frigate.h

@@ -43,7 +43,7 @@ class Frigate final : public AirUnit
     void deploy(const Coord& newLocation) override;
 
 protected:
-    virtual void turn();
+    virtual void turn() override;
 
 private:
     bool    droppedOffCargo;    ///< Is the cargo already dropped off?

include/units/Ornithopter.h

@@ -42,7 +42,7 @@ class Ornithopter final : public AirUnit
 protected:
     virtual FixPoint getDestinationAngle() const override;
 
-    virtual bool attack();
+    virtual bool attack() override;
 
 private:
     Uint32 timeLastShot;

src/Bullet.cpp

@@ -323,22 +323,22 @@ void Bullet::update()
         FixPoint angleToDestinationRad = destinationAngleRad(Coord(lround(realX), lround(realY)), destination);
         FixPoint angleToDestination = RadToDeg256(angleToDestinationRad);
 
-        FixPoint angleDiff = angleToDestination - angle;
-        if(angleDiff > 128) {
-            angleDiff -= 256;
-        } else if(angleDiff < -128) {
-            angleDiff += 256;
+        FixPoint angleDifference = angleToDestination - angle;
+        if(angleDifference > 128) {
+            angleDifference -= 256;
+        } else if(angleDifference < -128) {
+            angleDifference += 256;
         }
 
         static const FixPoint turnSpeed = 4.5_fix;
 
-        if(angleDiff >= turnSpeed) {
-            angleDiff = turnSpeed;
-        } else if(angleDiff <= -turnSpeed) {
-            angleDiff = -turnSpeed;
+        if(angleDifference >= turnSpeed) {
+            angleDifference = turnSpeed;
+        } else if(angleDifference <= -turnSpeed) {
+            angleDifference = -turnSpeed;
         }
 
-        angle += angleDiff;
+        angle += angleDifference;
 
         if(angle < 0) {
             angle += 256;

src/FileClasses/FileManager.cpp

@@ -107,7 +107,6 @@ std::vector<std::string> FileManager::getNeededFiles() {
 
 std::vector<std::string> FileManager::getMissingFiles() {
     std::vector<std::string> MissingFiles;
-    std::vector<std::string> searchPath = getSearchPath();
 
     for(const std::string& fileName : getNeededFiles()) {
         bool bFound = false;

src/FileClasses/POFile.cpp

@@ -99,7 +99,6 @@ std::map<std::string, std::string> loadPOFile(SDL_RWops* rwop, const std::string
             } else if(msgstrMode == true) {
                 // we have finished the previous translation
                 mapping[msgid] = msgstr;
-                msgid = "";
                 msgstr = "";
 
                 msgstrMode = false;

src/FileClasses/Pakfile.cpp

@@ -364,6 +364,7 @@ void Pakfile::readIndex()
 
         //pak-files are always little endian encoded
         newEntry.startOffset = SDL_SwapLE32(newEntry.startOffset);
+        newEntry.endOffset = 0;
 
         if(newEntry.startOffset == 0) {
             break;

src/FileClasses/Vocfile.cpp

@@ -166,12 +166,11 @@ static sdl2::sdl_ptr<Uint8[]> LoadVOC_RW(SDL_RWops* rwop, Uint32 &decsize, Uint3
                 //SDL_Log("VOC Data Block: Rate: %d, Packing: %d, Length: %d", rate, packing, len);
 
                 if (packing == 0) {
-                    Uint8* tmp = (Uint8 *) SDL_realloc(ret_sound.release(), decsize + len);
-                    if(tmp == nullptr) {
+                    Uint8* tmp_ret_sound = (Uint8 *) SDL_realloc(ret_sound.release(), decsize + len);
+                    if(tmp_ret_sound == nullptr) {
                         THROW(std::runtime_error, "LoadVOC_RW(): %s", strerror(errno));
                     } else {
-                        ret_sound.release();
-                        ret_sound = sdl2::sdl_ptr<Uint8[]> { tmp };
+                        ret_sound = sdl2::sdl_ptr<Uint8[]> { tmp_ret_sound };
                     }
 
                     if(SDL_RWread(rwop,ret_sound.get() + decsize,1,len) != len) {
@@ -207,12 +206,11 @@ static sdl2::sdl_ptr<Uint8[]> LoadVOC_RW(SDL_RWops* rwop, Uint32 &decsize, Uint3
                     length = SilenceLength;
                 }
 
-                Uint8* tmp = (Uint8 *) SDL_realloc(ret_sound.release(), decsize + length);
-                if(tmp == nullptr) {
+                Uint8* tmp_ret_sound = (Uint8 *) SDL_realloc(ret_sound.release(), decsize + length);
+                if(tmp_ret_sound == nullptr) {
                     THROW(std::runtime_error, "LoadVOC_RW(): %s", strerror(errno));
                 } else {
-                    ret_sound.release();
-                    ret_sound = sdl2::sdl_ptr<Uint8[]> { tmp };
+                    ret_sound = sdl2::sdl_ptr<Uint8[]> { tmp_ret_sound };
                 }
 
                 memset(ret_sound.get() + decsize,0x80,length);

src/FileClasses/music/DirectoryPlayer.cpp

@@ -51,9 +51,9 @@ DirectoryPlayer::DirectoryPlayer() : MusicPlayer(settings.audio.playMusic, setti
                                                                     };
 
     for(int i=0;i<MUSIC_NUM_MUSIC_TYPES;i++) {
-        char tmp[FILENAME_MAX];
+        char tmp2[FILENAME_MAX];
         const char* dirName =  musicDirectoryNames[i] + 1; // skip '/' at the beginning
-        fnkdat(dirName, tmp, FILENAME_MAX, FNKDAT_USER | FNKDAT_CREAT);
+        fnkdat(dirName, tmp2, FILENAME_MAX, FNKDAT_USER | FNKDAT_CREAT);
         musicFileList[i] = getMusicFileNames(configfilepath + musicDirectoryNames[i]);
     }
 

src/GUI/dune/BuilderList.cpp

@@ -262,17 +262,17 @@ void BuilderList::draw(Point position) {
                     }
 
                     if(bSoldOut) {
-                        SDL_Rect drawLocation = calcDrawingRect(pSoldOutTextTexture.get(), dest.x + BUILDERBTN_WIDTH/2, dest.y + BUILDERBTN_HEIGHT/2, HAlign::Center, VAlign::Center);
-                        SDL_RenderCopy(renderer, pSoldOutTextTexture.get(), nullptr, &drawLocation);
+                        SDL_Rect drawLocationSoldOut = calcDrawingRect(pSoldOutTextTexture.get(), dest.x + BUILDERBTN_WIDTH/2, dest.y + BUILDERBTN_HEIGHT/2, HAlign::Center, VAlign::Center);
+                        SDL_RenderCopy(renderer, pSoldOutTextTexture.get(), nullptr, &drawLocationSoldOut);
                     }
 
                 } else if(currentGame->getGameInitSettings().getGameOptions().onlyOnePalace && buildItem.itemID == Structure_Palace && pBuilder->getOwner()->getNumItems(Structure_Palace) > 0) {
 
                     SDL_Rect progressBar = { dest.x, dest.y, BUILDERBTN_WIDTH, BUILDERBTN_HEIGHT };
                     renderFillRect(renderer, &progressBar, COLOR_HALF_TRANSPARENT);
 
-                    SDL_Rect drawLocation = calcDrawingRect(pAlreadyBuiltTextTexture.get(), dest.x + BUILDERBTN_WIDTH/2, dest.y + BUILDERBTN_HEIGHT/2, HAlign::Center, VAlign::Center);
-                    SDL_RenderCopy(renderer, pAlreadyBuiltTextTexture.get(), nullptr, &drawLocation);
+                    SDL_Rect drawLocationAlreadyBuilt = calcDrawingRect(pAlreadyBuiltTextTexture.get(), dest.x + BUILDERBTN_WIDTH/2, dest.y + BUILDERBTN_HEIGHT/2, HAlign::Center, VAlign::Center);
+                    SDL_RenderCopy(renderer, pAlreadyBuiltTextTexture.get(), nullptr, &drawLocationAlreadyBuilt);
                 } else if(buildItem.itemID == pBuilder->getCurrentProducedItem()) {
                     FixPoint progress = pBuilder->getProductionProgress();
                     FixPoint price = buildItem.price;
@@ -282,22 +282,22 @@ void BuilderList::draw(Point position) {
                     renderFillRect(renderer, &progressBar, COLOR_HALF_TRANSPARENT);
 
                     if(pBuilder->isWaitingToPlace()) {
-                        SDL_Rect drawLocation = calcDrawingRect(pPlaceItTextTexture.get(), dest.x + BUILDERBTN_WIDTH/2, dest.y + BUILDERBTN_HEIGHT/2, HAlign::Center, VAlign::Center);
-                        SDL_RenderCopy(renderer, pPlaceItTextTexture.get(), nullptr, &drawLocation);
+                        SDL_Rect drawLocationWaitingToPlace = calcDrawingRect(pPlaceItTextTexture.get(), dest.x + BUILDERBTN_WIDTH/2, dest.y + BUILDERBTN_HEIGHT/2, HAlign::Center, VAlign::Center);
+                        SDL_RenderCopy(renderer, pPlaceItTextTexture.get(), nullptr, &drawLocationWaitingToPlace);
                     } else if(pBuilder->isOnHold()) {
-                        SDL_Rect drawLocation = calcDrawingRect(pOnHoldTextTexture.get(), dest.x + BUILDERBTN_WIDTH/2, dest.y + BUILDERBTN_HEIGHT/2, HAlign::Center, VAlign::Center);
-                        SDL_RenderCopy(renderer, pOnHoldTextTexture.get(), nullptr, &drawLocation);
+                        SDL_Rect drawLocationOnHold = calcDrawingRect(pOnHoldTextTexture.get(), dest.x + BUILDERBTN_WIDTH/2, dest.y + BUILDERBTN_HEIGHT/2, HAlign::Center, VAlign::Center);
+                        SDL_RenderCopy(renderer, pOnHoldTextTexture.get(), nullptr, &drawLocationOnHold);
                     } else if(pBuilder->isUnitLimitReached(buildItem.itemID)) {
-                        SDL_Rect drawLocation = calcDrawingRect(pUnitLimitReachedTextTexture.get(), dest.x + BUILDERBTN_WIDTH/2, dest.y + BUILDERBTN_HEIGHT/2, HAlign::Center, VAlign::Center);
-                        SDL_RenderCopy(renderer, pUnitLimitReachedTextTexture.get(), nullptr, &drawLocation);
+                        SDL_Rect drawLocationUnitLimitReached = calcDrawingRect(pUnitLimitReachedTextTexture.get(), dest.x + BUILDERBTN_WIDTH/2, dest.y + BUILDERBTN_HEIGHT/2, HAlign::Center, VAlign::Center);
+                        SDL_RenderCopy(renderer, pUnitLimitReachedTextTexture.get(), nullptr, &drawLocationUnitLimitReached);
                     }
                 }
 
                 if(buildItem.num > 0) {
                     // draw number of this in build list
                     sdl2::texture_ptr pNumberTexture = pFontManager->createTextureWithText(fmt::sprintf("%d", buildItem.num), COLOR_RED, FONT_STD12);
-                    SDL_Rect drawLocation = calcDrawingRect(pNumberTexture.get(), dest.x + BUILDERBTN_WIDTH - 3, dest.y + BUILDERBTN_HEIGHT + 2, HAlign::Right, VAlign::Bottom);
-                    SDL_RenderCopy(renderer, pNumberTexture.get(), nullptr, &drawLocation);
+                    SDL_Rect drawLocationNumber = calcDrawingRect(pNumberTexture.get(), dest.x + BUILDERBTN_WIDTH - 3, dest.y + BUILDERBTN_HEIGHT + 2, HAlign::Right, VAlign::Bottom);
+                    SDL_RenderCopy(renderer, pNumberTexture.get(), nullptr, &drawLocationNumber);
                 }
             }
 

src/INIMap/INIMapPreviewCreator.cpp

@@ -188,10 +188,10 @@ sdl2::surface_ptr INIMapPreviewCreator::createMinimapImageOfMap(int borderWidth,
         if(BloomString != "") {
             std::vector<std::string> BloomPositions  = splitString(BloomString);
 
-            for(unsigned int i=0; i < BloomPositions.size();i++) {
+            for(const std::string& strBloomPos : BloomPositions) {
                 // set bloom
                 int BloomPos;
-                if(parseString(BloomPositions[i], BloomPos)) {
+                if(parseString(strBloomPos, BloomPos)) {
                     int xpos = BloomPos % logicalSizeX - logicalOffsetX;
                     int ypos = BloomPos / logicalSizeX - logicalOffsetY;
                     if(xpos >= 0 && xpos < sizeX && ypos >= 0 && ypos < sizeY) {
@@ -212,10 +212,10 @@ sdl2::surface_ptr INIMapPreviewCreator::createMinimapImageOfMap(int borderWidth,
         if(SpecialString != "") {
             std::vector<std::string> SpecialPositions  = splitString(SpecialString);
 
-            for(unsigned int i=0; i < SpecialPositions.size();i++) {
+            for(const std::string& strSpecialPos : SpecialPositions) {
                 // set bloom
                 int SpecialPos;
-                if(parseString(SpecialPositions[i], SpecialPos)) {
+                if(parseString(strSpecialPos, SpecialPos)) {
                     int xpos = SpecialPos % logicalSizeX - logicalOffsetX;
                     int ypos = SpecialPos / logicalSizeX - logicalOffsetY;
                     if(xpos >= 0 && xpos < sizeX && ypos >= 0 && ypos < sizeY) {

src/players/QuantBot.cpp

@@ -508,8 +508,7 @@ void QuantBot::onDamage(const ObjectBase* pObject, int damage, Uint32 damagerID)
             doSetAttackMode(pGroundUnit, AREAGUARD);
             doMove2Pos(pGroundUnit, squadCenterLocation.x, squadCenterLocation.y, true);
         } else if(  (currentGame->techLevel > 3)
-                    && (pGroundUnit->getItemID() == Unit_RaiderTrike)
-                    && (pGroundUnit->getItemID() == Unit_Trike)
+                    && ((pGroundUnit->getItemID() == Unit_RaiderTrike) || (pGroundUnit->getItemID() == Unit_Trike))
                     && !pDamager->isInfantry()
                     && (pDamager->getItemID() != Unit_RaiderTrike)
                     && (pDamager->getItemID() != Unit_Trike)) {
@@ -1033,7 +1032,7 @@ void QuantBot::build(int militaryValue) {
                            && pBuilder->isAvailableToBuild(Unit_Soldier)
                            && gameMode == GameMode::Campaign
                            && ((itemCount[Structure_HeavyFactory] == 0) || militaryValue < militaryValueLimit * 0.30_fix)
-                           && itemCount[Structure_WOR == 0]
+                           && itemCount[Structure_WOR] == 0
                            && money > 1000
                            && pBuilder->getProductionQueueSize() < 1
                            && pBuilder->getBuildListSize() > 0

src/structures/RepairYard.cpp

@@ -153,7 +153,7 @@ void RepairYard::updateStructureSpecificStuff() {
             if(pCarryall != nullptr) {
                 pCarryall->setTarget(this);
                 pCarryall->clearPath();
-                ((GroundUnit*)pRepairUnit)->bookCarrier(pCarryall);
+                static_cast<GroundUnit*>(pRepairUnit)->bookCarrier(pCarryall);
                 pRepairUnit->setTarget(nullptr);
                 pRepairUnit->setDestination(pRepairUnit->getGuardPoint());
             } else {

src/structures/StarPort.cpp

@@ -187,7 +187,7 @@ void StarPort::doCancelItem(Uint32 itemID, bool multipleMode) {
 
 void StarPort::doPlaceOrder() {
 
-    if (currentProductionQueue.size() > 0) {
+    if (!currentProductionQueue.empty()) {
 
         if(currentGame->getGameInitSettings().getGameOptions().instantBuild == true) {
             arrivalTimer = 1;