Fixes for suggestions from clang-tidy.

src/cheat/CheatManager.cxx

@@ -224,7 +224,7 @@ void CheatManager::loadCheatDatabase()
   // Loop reading cheats
   while(getline(in, line))
   {
-    if(line.length() == 0)
+    if(line.empty())
       continue;
 
     const string::size_type one = line.find('\"', 0);

src/common/FBBackendSDL2.cxx

@@ -443,8 +443,8 @@ bool FBBackendSDL2::createRenderer()
 
     if(myRenderer == nullptr)
     {
-      const string msg = "ERROR: Unable to create SDL renderer: " + string(SDL_GetError());
-      Logger::error(msg);
+      Logger::error("ERROR: Unable to create SDL renderer: " +
+                    string{SDL_GetError()});
       return false;
     }
   }

src/common/PNGLibrary.cxx

@@ -359,8 +359,9 @@ void PNGLibrary::takeSnapshot(uInt32 number)
     try
     {
       Common::Rect rect;
-      const FBSurface& surface = myOSystem.frameBuffer().tiaSurface().baseSurface(rect);
-      myOSystem.png().saveImage(filename, surface, rect, metaData);
+      const FBSurface& surface =
+        myOSystem.frameBuffer().tiaSurface().baseSurface(rect);
+      PNGLibrary::saveImage(filename, surface, rect, metaData);
     }
     catch(const runtime_error& e)
     {
@@ -375,7 +376,7 @@ void PNGLibrary::takeSnapshot(uInt32 number)
 
     try
     {
-      myOSystem.png().saveImage(filename, metaData);
+      PNGLibrary::saveImage(filename, metaData);
     }
     catch(const runtime_error& e)
     {

src/common/SoundSDL2.cxx

@@ -475,7 +475,7 @@ void SoundSDL2::WavHandlerSDL2::processWav(uInt8* stream, uInt32 len)
       const int newFreq =
         std::round(static_cast<double>(mySpec.freq) * origLen / len);
 
-      if(static_cast<uInt32>(len) > myRemaining)
+      if(len > myRemaining)
         len = myRemaining;
 
       SDL_AudioCVT cvt;
@@ -501,7 +501,7 @@ void SoundSDL2::WavHandlerSDL2::processWav(uInt8* stream, uInt32 len)
     }
     else
     {
-      if(static_cast<uInt32>(len) > myRemaining)
+      if(len > myRemaining)
         len = myRemaining;
 
       // Mix volume adjusted WAV data into silent buffer

src/common/main.cxx

@@ -333,7 +333,8 @@ int main(int ac, char* av[])
     if(!localOpts["break"].toString().empty())
     {
       Debugger& dbg = theOSystem->debugger();
-      const uInt16 bp = uInt16(dbg.stringToValue(localOpts["break"].toString()));
+      const uInt16 bp =
+        static_cast<uInt16>(dbg.stringToValue(localOpts["break"].toString()));
       dbg.setBreakPoint(bp);
     }
 #endif

src/common/repository/CompositeKVRJsonAdapter.cxx

@@ -73,5 +73,5 @@ bool CompositeKVRJsonAdapter::has(string_view key)
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void CompositeKVRJsonAdapter::remove(string_view key)
 {
-  return myKvr.remove(key);
+  myKvr.remove(key);
 }

src/common/repository/KeyValueRepositoryConfigfile.cxx

@@ -38,7 +38,7 @@ KVRMap KeyValueRepositoryConfigfile::load(istream& in)
       line.erase(garbage, 1);
 
     // Ignore commented and empty lines
-    if((line.length() == 0) || (line[0] == ';'))
+    if(line.empty() || (line[0] == ';'))
       continue;
 
     // Search for the equal sign and discard the line if its not found
@@ -50,7 +50,7 @@ KVRMap KeyValueRepositoryConfigfile::load(istream& in)
     value = BSPF::trim(line.substr(equalPos + 1, line.length() - key.length() - 1));
 
     // Skip absent key
-    if(key.length() == 0)
+    if(key.empty())
       continue;
 
     values[key] = value;

src/debugger/CartDebug.cxx

@@ -819,7 +819,7 @@ string CartDebug::loadListFile()
       // We need to read the address as a string, since it may contain 'U'
       int addr = -1;
       buf >> addr >> addr_s;
-      if(addr_s.length() == 0)
+      if(addr_s.empty())
         continue;
 
       addr = BSPF::stoi<16>(addr_s[0] == 'U' ? addr_s.substr(1) : addr_s);
@@ -877,7 +877,7 @@ string CartDebug::loadSymbolFile()
     stringstream buf(label);
     buf >> label >> hex >> value;
 
-    if(label.length() > 0 && label[0] != '-' && value >= 0)
+    if(!label.empty() && label[0] != '-' && value >= 0)
     {
       // Make sure the value doesn't represent a constant
       // For now, we simply ignore constants completely

src/debugger/DebuggerParser.cxx

@@ -773,7 +773,7 @@ string DebuggerParser::saveScriptFile(string file)
 
   const FSNode node(file);
 
-  if(node.exists() || out.str().length())
+  if(node.exists() || !out.str().empty())
   {
     try
     {
@@ -1375,7 +1375,7 @@ void DebuggerParser::executeDump()
       {
         if(OK)
         {
-          const stringstream  localOut(outStr);
+          const stringstream localOut(outStr);
           ostringstream localResult(resultStr, std::ios_base::app);
 
           saveDump(node, localOut, localResult);

src/debugger/gui/PromptWidget.cxx

@@ -124,7 +124,7 @@ void PromptWidget::handleMouseWheel(int x, int y, int direction)
 void PromptWidget::printPrompt()
 {
   const string watches = instance().debugger().showWatches();
-  if(watches.length() > 0)
+  if(!watches.empty())
     print(watches);
 
   print(PROMPT);

src/debugger/gui/RamWidget.cxx

@@ -396,7 +396,7 @@ string RamWidget::doSearch(string_view str)
 {
   bool comparisonSearch = true;
 
-  if(str.length() == 0)
+  if(str.empty())
   {
     // An empty field means return all memory locations
     comparisonSearch = false;
@@ -454,7 +454,7 @@ string RamWidget::doCompare(string_view str)
   bool comparativeSearch = false;
   int searchVal = 0, offset = 0;
 
-  if(str.length() == 0)
+  if(str.empty())
     return "Enter an absolute or comparative value";
 
   // Do some pre-processing on the string

src/debugger/yacc/YaccParser.cxx

@@ -25,7 +25,7 @@ namespace YaccParser {
 
 #include "y.tab.h"
 
-enum class State {
+enum class State : uInt8 {
   DEFAULT,
   IDENTIFIER,
   OPERATOR,
@@ -85,12 +85,12 @@ int parse(const string& in)
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 // hand-rolled lexer. Hopefully faster than flex...
-inline constexpr bool is_base_prefix(char x)
+constexpr bool is_base_prefix(char x)
 {
   return ( (x=='\\' || x=='$' || x=='#') );
 }
 
-inline constexpr bool is_identifier(char x)
+constexpr bool is_identifier(char x)
 {
   return ( (x>='0' && x<='9') ||
            (x>='a' && x<='z') ||
@@ -98,7 +98,7 @@ inline constexpr bool is_identifier(char x)
             x=='.' || x=='_'  );
 }
 
-inline constexpr bool is_operator(char x)
+constexpr bool is_operator(char x)
 {
   return ( (x=='+' || x=='-' || x=='*' ||
             x=='/' || x=='<' || x=='>' ||

src/emucore/CartMVC.cxx

@@ -63,7 +63,7 @@ class StreamReader : public Serializable
       return static_cast<bool>(myFile);
     }
 
-    bool isValid() const {
+    [[nodiscard]] bool isValid() const {
       return myFileSize > 0;
     }
 
@@ -812,7 +812,7 @@ class MovieCart : public Serializable
       Last = Time
     };
 
-    enum class TitleState
+    enum class TitleState : uInt8
     {
       Display,
       Exiting,

src/emucore/FBSurface.cxx

@@ -347,7 +347,7 @@ int FBSurface::drawString(const GUI::Font& font, string_view s,
   string inStr{s};
 
   // draw multiline string
-  while(inStr.length() && h >= font.getFontHeight() * 2)
+  while(!inStr.empty() && h >= font.getFontHeight() * 2)
   {
     // String is too wide.
     string leftStr, rightStr;
@@ -363,7 +363,7 @@ int FBSurface::drawString(const GUI::Font& font, string_view s,
     inStr = rightStr;
     lines++;
   }
-  if(inStr.length())
+  if(!inStr.empty())
   {
     drawString(font, inStr, x, y, w, color, align, deltax, useEllipsis, shadowColor,
                linkStart, linkLen, underline);

src/emucore/FrameBuffer.cxx

@@ -326,8 +326,8 @@ FBInitStatus FrameBuffer::createDisplay(string_view title, BufferType type,
   if(myOSystem.eventHandler().inTIAMode())
   {
     // Phosphor mode can be enabled either globally or per-ROM
-    int p_blend;
-    bool enable;
+    int p_blend = 0;
+    bool enable = false;
     const int phosphorMode = PhosphorHandler::toPhosphorMode(
       myOSystem.settings().getString(PhosphorHandler::SETTING_MODE));
 

src/emucore/QuadTari.cxx

@@ -137,9 +137,9 @@ bool QuadTari::read(DigitalPin pin)
 void QuadTari::write(DigitalPin pin, bool value)
 {
   if(isFirst())
-    return myFirstController->write(pin, value);
+    myFirstController->write(pin, value);
   else
-    return mySecondController->write(pin, value);
+    mySecondController->write(pin, value);
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

src/emucore/tia/TIA.cxx

@@ -26,7 +26,7 @@
 #include "PhosphorHandler.hxx"
 #include "Base.hxx"
 
-enum CollisionMask: uInt32 {
+enum CollisionMask: uInt16 {
   player0   = 0b0111110000000000,
   player1   = 0b0100001111000000,
   missile0  = 0b0010001000111000,
@@ -1121,9 +1121,9 @@ bool TIA::toggleBit(TIABit b, uInt8 mode)
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 bool TIA::toggleBits(bool toggle)
 {
-  toggleBit(static_cast<TIABit>(0xFF), toggle
-                          ? mySpriteEnabledBits > 0 ? 0 : 1
-                          : mySpriteEnabledBits);
+  toggleBit(TIABit::AllBits, toggle
+    ? mySpriteEnabledBits > 0 ? 0 : 1
+    : mySpriteEnabledBits);
 
   return mySpriteEnabledBits;
 }
@@ -1166,9 +1166,9 @@ bool TIA::toggleCollision(TIABit b, uInt8 mode)
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 bool TIA::toggleCollisions(bool toggle)
 {
-  toggleCollision(static_cast<TIABit>(0xFF), toggle
-                                ? myCollisionsEnabledBits > 0 ? 0 : 1
-                                : myCollisionsEnabledBits);
+  toggleCollision(TIABit::AllBits, toggle
+    ? myCollisionsEnabledBits > 0 ? 0 : 1
+    : myCollisionsEnabledBits);
 
   return myCollisionsEnabledBits;
 }
@@ -1433,8 +1433,7 @@ void TIA::onFrameComplete()
       int count = 0;
       for(uInt32 y = 0; y <= myFrameEnd; ++y)
       {
-        int delta;
-        delta = std::abs(myPosP0[y][myFlickerFrame] - myPosP0[y][otherFrame]);
+        int delta = std::abs(myPosP0[y][myFlickerFrame] - myPosP0[y][otherFrame]);
         if(delta >= MIN_FLICKER_DELTA && delta <= MAX_FLICKER_DELTA)
           ++count;
         delta = std::abs(myPosP1[y][myFlickerFrame] - myPosP1[y][otherFrame]);
@@ -1684,8 +1683,10 @@ FORCE_INLINE void TIA::nextLine()
       if(myBall.isOn())
         myPosBL[y][myFlickerFrame] = myBall.getPosition();
       // Note: code checks only right side of playfield
-      myPatPF[y][myFlickerFrame] = (uInt32(registerValue(PF0))) << 16
-        | (uInt32(registerValue(PF1))) << 8 | uInt32(registerValue(PF2));
+      myPatPF[y][myFlickerFrame] =
+          (static_cast<uInt32>(registerValue(PF0))) << 16
+        | (static_cast<uInt32>(registerValue(PF1))) << 8
+        | (static_cast<uInt32>(registerValue(PF2)));
       // Define end of frame for faster auto-phosphor calculation
       if(!cloned)
         myFrameEnd = y;

src/emucore/tia/TIAConstants.hxx

@@ -36,7 +36,7 @@ namespace TIAConstants {
     H_BLANK_CLOCKS = H_CLOCKS - H_PIXEL;  // = 68
 }
 
-enum TIABit {
+enum TIABit: uInt8 {
   P0Bit       = 0x01,  // Bit for Player 0
   M0Bit       = 0x02,  // Bit for Missle 0
   P1Bit       = 0x04,  // Bit for Player 1
@@ -44,7 +44,8 @@ enum TIABit {
   BLBit       = 0x10,  // Bit for Ball
   PFBit       = 0x20,  // Bit for Playfield
   ScoreBit    = 0x40,  // Bit for Playfield score mode
-  PriorityBit = 0x80   // Bit for Playfield priority
+  PriorityBit = 0x80,  // Bit for Playfield priority
+  AllBits     = 0xff
 };
 
 enum TIAColor {

src/gui/ContextMenu.cxx

@@ -369,9 +369,13 @@ void ContextMenu::sendSelection()
   if(_showScroll)
   {
     if(_selectedOffset == 0)  // scroll up
-      return scrollUp();
+    {
+      scrollUp();  return;
+    }
     else if(_selectedOffset == _numEntries+1) // scroll down
-      return scrollDown();
+    {
+      scrollDown();  return;
+    }
     else if(_isScrolling)
       return;
     else

src/gui/FileListWidget.cxx

@@ -243,7 +243,7 @@ bool FileListWidget::hasNextHistory()
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 string& FileListWidget::fixPath(string& path)
 {
-  if(path.length() > 0 && path.back() == FSNode::PATH_SEPARATOR)
+  if(!path.empty() && path.back() == FSNode::PATH_SEPARATOR)
   {
     path.pop_back();
     if(path.length() == 2 && path.back() == ':')
@@ -255,7 +255,8 @@ string& FileListWidget::fixPath(string& path)
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void FileListWidget::addHistory(const FSNode& node)
 {
-  if (!_history.empty()) {
+  if(!_history.empty())
+  {
     while(_currentHistory != std::prev(_history.end(), 1))
       _history.pop_back();
 

src/gui/GameInfoDialog.cxx

@@ -473,7 +473,7 @@ void GameInfoDialog::addCartridgeTab()
   wid.push_back(myNote);
 
   ypos += lineHeight + VGAP;
-  int bw = buttonWidth(">");
+  const int bw = buttonWidth(">");
   new StaticTextWidget(myTab, _font, xpos, ypos + 1, lwidth, fontHeight, "Link");
   myUrl = new EditTextWidget(myTab, _font, xpos + lwidth, ypos - 1,
                              fwidth - bw - HGAP, lineHeight, "");

src/gui/HelpDialog.cxx

@@ -106,9 +106,9 @@ void HelpDialog::updateStrings(uInt8 page, uInt8 lines, string& title)
   const auto ADD_EVENT = [&](const Event::Type e, const string & d)
   {
     string desc = instance().eventHandler().getMappingDesc(e, EventMode::kEmulationMode);
-    if(!desc.length())
+    if(desc.empty())
       desc = instance().eventHandler().getMappingDesc(e, EventMode::kMenuMode);
-    ADD_BIND(desc.length() ? desc : "None", d);
+    ADD_BIND(!desc.empty() ? desc : "None", d);
   };
   const auto ADD_TEXT = [&](string_view d) { ADD_BIND("", d); };
   const auto ADD_LINE = [&]() { ADD_BIND("", ""); };

src/gui/LauncherDialog.cxx

@@ -206,7 +206,6 @@ void LauncherDialog::addFilteringWidgets(int& ypos)
     // Show the files counter
     myRomCount = new StaticTextWidget(this, _font, xpos, ypos,
                                       lwFound, fontHeight, "", TextAlign::Right);
-    xpos = myRomCount->getRight() + LBL_GAP;
     xpos = _w - HBORDER - bwSettings - randomButtonWidth - btnGap;
 
     // Show the random ROM button

src/gui/RomImageWidget.cxx

@@ -577,7 +577,7 @@ void RomImageWidget::drawWidget(bool hilite)
   const int wText = _font.getStringWidth(buf.str()) + 8;
 
   s.fillRect(_x, yText, _w, _font.getFontHeight(), _bgcolor);
-  if(myLabel.length())
+  if(!myLabel.empty())
     s.drawString(_font, myLabel, _x + 8, yText, _w - wText - 16 - _font.getMaxCharWidth() * 2, _textcolor);
   if(!myImageList.empty())
     s.drawString(_font, buf.str(), _x + _w - wText, yText, wText, _textcolor);

src/gui/UndoHandler.cxx

@@ -102,5 +102,5 @@ uInt32 UndoHandler::lastDiff(string_view text, string_view oldText)
       break;
     pos--;
   }
-  return static_cast<uInt32>(pos);
+  return pos;
 }

src/gui/WhatsNewDialog.cxx

@@ -104,7 +104,7 @@ void WhatsNewDialog::add(int& ypos, string_view text)
   {
     int i = MAX_CHARS;
 
-    while(--i && txt[i] != ' ');
+    while(--i && txt[i] != ' ');  // NOLINT: bugprone-inc-dec-in-conditions
     new StaticTextWidget(this, _font, HBORDER, ypos, txt.substr(0, i));
     txt = " " + txt.substr(i);
     ypos += fontHeight;

src/gui/Widget.cxx

@@ -1114,8 +1114,8 @@ void SliderWidget::handleMouseMoved(int x, int y)
   // TODO: when the mouse is dragged outside the widget, the slider should
   // snap back to the old value.
   if(isEnabled() && _isDragging &&
-     x >= static_cast<int>(_labelWidth - 4) &&
-     x <= static_cast<int>(_w - _valueLabelGap - _valueLabelWidth + 4))
+     x >= (_labelWidth - 4) &&
+     x <= (_w - _valueLabelGap - _valueLabelWidth + 4))
     setValue(posToValue(x - _labelWidth));
 }
 

src/os/unix/FSNodePOSIX.cxx

@@ -32,7 +32,7 @@ FSNodePOSIX::FSNodePOSIX()
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 FSNodePOSIX::FSNodePOSIX(string_view path, bool verify)
-  : _path{path.length() > 0 ? path : "~"}  // Default to home directory
+  : _path{!path.empty() ? path : "~"}  // Default to home directory
 {
   // Expand '~' to the HOME environment variable
   if (_path[0] == '~')
@@ -64,7 +64,7 @@ bool FSNodePOSIX::setFlags()
     _size = st.st_size;
 
     // Add a trailing slash, if necessary
-    if (_isDirectory && _path.length() > 0 &&
+    if (_isDirectory && !_path.empty() &&
         _path.back() != FSNode::PATH_SEPARATOR)
       _path += FSNode::PATH_SEPARATOR;
 
@@ -142,7 +142,7 @@ bool FSNodePOSIX::getChildren(AbstractFSList& myList, ListMode mode) const
       continue;
 
     string newPath(_path);
-    if (newPath.length() > 0 && newPath.back() != FSNode::PATH_SEPARATOR)
+    if (!newPath.empty() && newPath.back() != FSNode::PATH_SEPARATOR)
       newPath += FSNode::PATH_SEPARATOR;
     newPath += dp->d_name;