style: fix all cppcheck warnings

- Fix ignored return value in TxtReaderActivity - Remove unused variables across multiple files - Add const correctness to DictionaryMargins and EpubReaderActivity - Replace inefficient substr patterns with resize+append - Use STL algorithms (find_if, any_of, copy_if, transform) where applicable - Remove dead sync placeholder code in EpubReaderChapterSelectionActivity - Add cppcheck suppression for ValueFlow analysis limitation Resolves all low, medium, and high severity cppcheck defects.
2026-01-28 09:45:42 -05:00 · 2026-01-28 09:45:42 -05:00 · bd95bfd44d
commit bd95bfd44d
parent fe446d4690
13 changed files with 66 additions and 102 deletions
--- a/src/activities/boot_sleep/SleepActivity.cpp
+++ b/src/activities/boot_sleep/SleepActivity.cpp
@ -164,8 +164,7 @@ void SleepActivity::renderBitmapSleepScreen(const Bitmap& bitmap, const std::str
  float cropX = 0, cropY = 0;
  int drawWidth = pageWidth;
  int drawHeight = pageHeight;
-  int fillWidth = pageWidth;   // Actual area the image will occupy
-  int fillHeight = pageHeight;
+  int fillWidth, fillHeight;  // Actual area the image will occupy (set per mode)

  Serial.printf("[%lu] [SLP] bitmap %d x %d, screen %d x %d\n", millis(), bitmap.getWidth(), bitmap.getHeight(),
                pageWidth, pageHeight);
@ -183,9 +182,9 @@ void SleepActivity::renderBitmapSleepScreen(const Bitmap& bitmap, const std::str
    // Don't constrain to screen dimensions - drawBitmap will clip
    drawWidth = 0;
    drawHeight = 0;
-    fillWidth = bitmap.getWidth();
-    fillHeight = bitmap.getHeight();
-    Serial.printf("[%lu] [SLP] ACTUAL mode: centering at %d, %d\n", millis(), x, y);
+    fillWidth = static_cast<int>(bitmap.getWidth());
+    fillHeight = static_cast<int>(bitmap.getHeight());
+    Serial.printf("[%lu] [SLP] ACTUAL mode: centering at %d, %d (fill: %dx%d)\n", millis(), x, y, fillWidth, fillHeight);
  } else if (coverMode == CrossPointSettings::SLEEP_SCREEN_COVER_MODE::CROP) {
    // CROP mode: Scale to fill screen completely (may crop edges)
    // Calculate crop values to fill the screen while maintaining aspect ratio
--- a/src/activities/dictionary/DictionaryMargins.h
+++ b/src/activities/dictionary/DictionaryMargins.h
@ -18,7 +18,7 @@
 * - PortraitInverted: Front=TOP, Side=LEFT
 * - LandscapeCCW: Front=RIGHT, Side=TOP
 */
-inline void getDictionaryContentMargins(GfxRenderer& renderer, int* outTop, int* outRight, int* outBottom,
+inline void getDictionaryContentMargins(const GfxRenderer& renderer, int* outTop, int* outRight, int* outBottom,
                                        int* outLeft) {
  // Start with same base margins as reader (getOrientedViewableTRBL + screenMargin)
  renderer.getOrientedViewableTRBL(outTop, outRight, outBottom, outLeft);
--- a/src/activities/dictionary/DictionaryResultActivity.cpp
+++ b/src/activities/dictionary/DictionaryResultActivity.cpp
@ -152,7 +152,6 @@ void DictionaryResultActivity::render() const {
  int marginTop, marginRight, marginBottom, marginLeft;
  getDictionaryContentMargins(renderer, &marginTop, &marginRight, &marginBottom, &marginLeft);

-  const auto pageWidth = renderer.getScreenWidth();
  const auto pageHeight = renderer.getScreenHeight();

  // Draw header with top margin
--- a/src/activities/dictionary/EpubWordSelectionActivity.cpp
+++ b/src/activities/dictionary/EpubWordSelectionActivity.cpp
@ -77,13 +77,9 @@ void EpubWordSelectionActivity::buildWordList() {
    while (wordIt != words.end() && xPosIt != xPositions.end() && styleIt != styles.end()) {
      // Skip whitespace-only words
      const std::string& wordText = *wordIt;
-      bool hasAlpha = false;
-      for (char c : wordText) {
-        if (std::isalpha(static_cast<unsigned char>(c))) {
-          hasAlpha = true;
-          break;
-        }
-      }
+      const bool hasAlpha = std::any_of(wordText.begin(), wordText.end(), [](char c) {
+        return std::isalpha(static_cast<unsigned char>(c));
+      });

      if (hasAlpha) {
        WordInfo info;
@ -106,7 +102,6 @@ void EpubWordSelectionActivity::buildWordList() {
 int EpubWordSelectionActivity::findLineForWordIndex(int wordIndex) const {
  if (wordIndex < 0 || wordIndex >= static_cast<int>(allWords.size())) return 0;

-  const int targetY = allWords[wordIndex].y;
  int lineIdx = 0;
  int lastY = -1;

--- a/src/activities/home/BookmarkListActivity.cpp
+++ b/src/activities/home/BookmarkListActivity.cpp
@ -113,7 +113,6 @@ void BookmarkListActivity::loop() {

  // Normal state handling
  const int itemCount = static_cast<int>(bookmarks.size());
-  const int pageItems = getPageItems();

  // Long press Confirm to delete bookmark
  if (mappedInput.isPressed(MappedInputManager::Button::Confirm) &&
--- a/src/activities/home/HomeActivity.cpp
+++ b/src/activities/home/HomeActivity.cpp
@ -691,7 +691,8 @@ void HomeActivity::render() {
    std::string truncatedLabel = listsLabel;
    const int maxLabelWidth = halfTileWidth - 16;  // Padding
    while (renderer.getTextWidth(UI_10_FONT_ID, truncatedLabel.c_str()) > maxLabelWidth && truncatedLabel.length() > 3) {
-      truncatedLabel = truncatedLabel.substr(0, truncatedLabel.length() - 4) + "...";
+      truncatedLabel.resize(truncatedLabel.length() - 4);
+      truncatedLabel += "...";
    }

    const int textWidth = renderer.getTextWidth(UI_10_FONT_ID, truncatedLabel.c_str());
--- a/src/activities/home/MyLibraryActivity.cpp
+++ b/src/activities/home/MyLibraryActivity.cpp
@ -7,6 +7,7 @@
 #include <algorithm>
 #include <cctype>
 #include <cstring>
+#include <iterator>
 #include <set>

 #include "BookListStore.h"
@ -154,12 +155,11 @@ void MyLibraryActivity::loadBookmarkedBooks() {
  
  // Try to get better metadata from recent books
  for (auto& book : bookmarkedBooks) {
-    for (const auto& recent : recentBooks) {
-      if (recent.path == book.path) {
-        if (!recent.title.empty()) book.title = recent.title;
-        if (!recent.author.empty()) book.author = recent.author;
-        break;
-      }
+    auto it = std::find_if(recentBooks.begin(), recentBooks.end(),
+                           [&book](const RecentBooksStore::RecentBook& recent) { return recent.path == book.path; });
+    if (it != recentBooks.end()) {
+      if (!it->title.empty()) book.title = it->title;
+      if (!it->author.empty()) book.author = it->author;
    }
  }
 }
@ -207,12 +207,11 @@ void MyLibraryActivity::loadAllBooks() {
          }
          
          // Try to get metadata from recent books if available
-          for (const auto& recent : recentBooks) {
-            if (recent.path == fullPath) {
-              if (!recent.title.empty()) result.title = recent.title;
-              if (!recent.author.empty()) result.author = recent.author;
-              break;
-            }
+          auto it = std::find_if(recentBooks.begin(), recentBooks.end(),
+                                 [&fullPath](const RecentBooksStore::RecentBook& recent) { return recent.path == fullPath; });
+          if (it != recentBooks.end()) {
+            if (!it->title.empty()) result.title = it->title;
+            if (!it->author.empty()) result.author = it->author;
          }
          
          allBooks.push_back(result);
@ -282,11 +281,9 @@ void MyLibraryActivity::buildSearchCharacters() {
  }
  
  // Add symbols (anything else in the set)
-  for (char c : charSet) {
-    if (!std::isalpha(static_cast<unsigned char>(c)) && !std::isdigit(static_cast<unsigned char>(c))) {
-      searchCharacters.push_back(c);
-    }
-  }
+  std::copy_if(charSet.begin(), charSet.end(), std::back_inserter(searchCharacters), [](char c) {
+    return !std::isalpha(static_cast<unsigned char>(c)) && !std::isdigit(static_cast<unsigned char>(c));
+  });
  
  // Reset character index if it's out of bounds
  if (searchCharIndex >= static_cast<int>(searchCharacters.size()) + 3) {  // +3 for special keys
@ -304,16 +301,20 @@ void MyLibraryActivity::updateSearchResults() {
  
  // Convert query to lowercase for case-insensitive matching
  std::string queryLower = searchQuery;
-  for (char& c : queryLower) c = tolower(c);
+  std::transform(queryLower.begin(), queryLower.end(), queryLower.begin(),
+                 [](unsigned char c) { return std::tolower(c); });
  
  for (const auto& book : allBooks) {
    // Convert title, author, and path to lowercase
    std::string titleLower = book.title;
    std::string authorLower = book.author;
    std::string pathLower = book.path;
-    for (char& c : titleLower) c = tolower(c);
-    for (char& c : authorLower) c = tolower(c);
-    for (char& c : pathLower) c = tolower(c);
+    std::transform(titleLower.begin(), titleLower.end(), titleLower.begin(),
+                   [](unsigned char c) { return std::tolower(c); });
+    std::transform(authorLower.begin(), authorLower.end(), authorLower.begin(),
+                   [](unsigned char c) { return std::tolower(c); });
+    std::transform(pathLower.begin(), pathLower.end(), pathLower.begin(),
+                   [](unsigned char c) { return std::tolower(c); });
    
    int score = 0;
    
@ -575,6 +576,7 @@ void MyLibraryActivity::executeListAction() {
  updateRequired = true;
 }

+// cppcheck-suppress checkLevelNormal
 void MyLibraryActivity::loop() {
  // Handle action menu state
  if (uiState == UIState::ActionMenu) {
@ -1372,7 +1374,6 @@ void MyLibraryActivity::renderRecentTab() const {
  const auto pageWidth = renderer.getScreenWidth();
  const int pageItems = getPageItems();
  const int bookCount = static_cast<int>(recentBooks.size());
-  const int totalItems = bookCount + 1;  // +1 for "Search..." shortcut

  // Calculate bezel-adjusted margins
  const int bezelTop = renderer.getBezelOffsetTop();
@ -1558,7 +1559,6 @@ void MyLibraryActivity::renderListsTab() const {
  const auto pageWidth = renderer.getScreenWidth();
  const int pageItems = getPageItems();
  const int listCount = static_cast<int>(lists.size());
-  const int totalItems = listCount + 1;  // +1 for "Search..." shortcut

  // Calculate bezel-adjusted margins
  const int bezelTop = renderer.getBezelOffsetTop();
@ -1610,7 +1610,6 @@ void MyLibraryActivity::renderFilesTab() const {
  const auto pageWidth = renderer.getScreenWidth();
  const int pageItems = getPageItems();
  const int fileCount = static_cast<int>(files.size());
-  const int totalItems = fileCount + 1;  // +1 for "Search..." shortcut

  // Calculate bezel-adjusted margins
  const int bezelTop = renderer.getBezelOffsetTop();
@ -1838,7 +1837,6 @@ void MyLibraryActivity::renderBookmarksTab() const {
  const auto pageWidth = renderer.getScreenWidth();
  const int pageItems = getPageItems();
  const int bookCount = static_cast<int>(bookmarkedBooks.size());
-  const int totalItems = bookCount + 1;  // +1 for "Search..." shortcut

  // Calculate bezel-adjusted margins
  const int bezelTop = renderer.getBezelOffsetTop();
@ -2043,7 +2041,6 @@ void MyLibraryActivity::renderCharacterPicker(int y) const {
  
  // Determine scroll offset to keep selected character visible
  int scrollOffset = 0;
-  int selectedX = 0;
  int currentX = 0;
  
  // Calculate position of selected item
@ -2061,9 +2058,8 @@ void MyLibraryActivity::renderCharacterPicker(int y) const {
    }
    
    if (i == searchCharIndex) {
-      selectedX = currentX;
      // Center the selected item in the visible area
-      scrollOffset = selectedX - availableWidth / 2 + itemWidth / 2;
+      scrollOffset = currentX - availableWidth / 2 + itemWidth / 2;
      if (scrollOffset < 0) scrollOffset = 0;
      if (scrollOffset > totalWidth - availableWidth) {
        scrollOffset = std::max(0, totalWidth - availableWidth);
--- a/src/activities/network/CrossPointWebServerActivity.cpp
+++ b/src/activities/network/CrossPointWebServerActivity.cpp
@ -640,7 +640,8 @@ void CrossPointWebServerActivity::renderWebBrowserScreen() const {

    std::string ssidInfo = "Network: " + connectedSSID;
    if (ssidInfo.length() > 35) {
-      ssidInfo = ssidInfo.substr(0, 32) + "...";
+      ssidInfo.resize(32);
+      ssidInfo += "...";
    }
    renderer.drawText(NOTOSANS_12_FONT_ID, TEXT_X, textY, ssidInfo.c_str());
    textY += LINE_SPACING;
@ -679,7 +680,8 @@ void CrossPointWebServerActivity::renderCompanionAppScreen() const {
  // Show network info
  std::string ssidInfo = "Network: " + connectedSSID;
  if (ssidInfo.length() > 35) {
-    ssidInfo = ssidInfo.substr(0, 32) + "...";
+    ssidInfo.resize(32);
+    ssidInfo += "...";
  }
  renderer.drawText(NOTOSANS_12_FONT_ID, TEXT_X, textY, ssidInfo.c_str());
  textY += LINE_SPACING;
@ -728,7 +730,8 @@ void CrossPointWebServerActivity::renderCompanionAppLibraryScreen() const {
  // Show network info
  std::string ssidInfo = "Network: " + connectedSSID;
  if (ssidInfo.length() > 35) {
-    ssidInfo = ssidInfo.substr(0, 32) + "...";
+    ssidInfo.resize(32);
+    ssidInfo += "...";
  }
  renderer.drawText(NOTOSANS_12_FONT_ID, TEXT_X, textY, ssidInfo.c_str());
  textY += LINE_SPACING;
--- a/src/activities/reader/EpubReaderActivity.cpp
+++ b/src/activities/reader/EpubReaderActivity.cpp
@ -386,9 +386,7 @@ void EpubReaderActivity::loop() {
        [this](QuickMenuAction action) {
          // Cache values before exitActivity
          EpubReaderActivity* self = this;
-          GfxRenderer& cachedRenderer = renderer;
-          MappedInputManager& cachedMappedInput = mappedInput;
-          Section* cachedSection = section.get();
+          const Section* cachedSection = section.get();
          SemaphoreHandle_t cachedMutex = renderingMutex;
          
          exitActivity();
@ -938,7 +936,8 @@ void EpubReaderActivity::renderEndOfBookPrompt() {
  // Book title (truncated if needed)
  std::string bookTitle = epub->getTitle();
  if (bookTitle.length() > 30) {
-    bookTitle = bookTitle.substr(0, 27) + "...";
+    bookTitle.resize(27);
+    bookTitle += "...";
  }
  renderer.drawCenteredText(UI_10_FONT_ID, 120, bookTitle.c_str());

--- a/src/activities/reader/EpubReaderChapterSelectionActivity.cpp
+++ b/src/activities/reader/EpubReaderChapterSelectionActivity.cpp
@ -10,7 +10,12 @@ namespace {
 constexpr int SKIP_PAGE_MS = 700;
 }  // namespace

-bool EpubReaderChapterSelectionActivity::hasSyncOption() const { return false; }
+// Sync feature is currently disabled - will be enabled when implemented
+// NOLINTNEXTLINE(readability-convert-member-functions-to-static)
+bool EpubReaderChapterSelectionActivity::hasSyncOption() const {
+  // TODO: Return true when sync credentials are configured
+  return false;
+}

 int EpubReaderChapterSelectionActivity::getTotalItems() const {
  // Add 2 for sync options (top and bottom) if credentials are configured
@ -18,12 +23,6 @@ int EpubReaderChapterSelectionActivity::getTotalItems() const {
  return epub->getTocItemsCount() + syncCount;
 }

-bool EpubReaderChapterSelectionActivity::isSyncItem(int index) const {
-  if (!hasSyncOption()) return false;
-  // First item and last item are sync options
-  return index == 0 || index == getTotalItems() - 1;
-}
-
 int EpubReaderChapterSelectionActivity::tocIndexFromItemIndex(int itemIndex) const {
  // Account for the sync option at the top
  const int offset = hasSyncOption() ? 1 : 0;
@ -94,10 +93,6 @@ void EpubReaderChapterSelectionActivity::onExit() {
  renderingMutex = nullptr;
 }

-void EpubReaderChapterSelectionActivity::launchSyncActivity() {
-  // KOReader sync functionality removed
-}
-
 void EpubReaderChapterSelectionActivity::loop() {
  if (subActivity) {
    subActivity->loop();
@ -114,13 +109,7 @@ void EpubReaderChapterSelectionActivity::loop() {
  const int totalItems = getTotalItems();

  if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) {
-    // Check if sync option is selected (first or last item)
-    if (isSyncItem(selectorIndex)) {
-      launchSyncActivity();
-      return;
-    }
-
-    // Get TOC index (account for top sync offset)
+    // Get TOC index (account for top sync offset if enabled)
    const int tocIndex = tocIndexFromItemIndex(selectorIndex);
    const auto newSpineIndex = epub->getSpineIndexForTocIndex(tocIndex);
    if (newSpineIndex == -1) {
@ -182,19 +171,13 @@ void EpubReaderChapterSelectionActivity::renderScreen() {
    const int displayY = 60 + bezelTop + (itemIndex % pageItems) * 30;
    const bool isSelected = (itemIndex == selectorIndex);

-    if (isSyncItem(itemIndex)) {
-      // Draw sync option (at top or bottom)
-      renderer.drawText(UI_10_FONT_ID, 20 + bezelLeft, displayY, ">> Sync Progress", !isSelected);
-    } else {
-      // Draw TOC item (account for top sync offset)
-      const int tocIndex = tocIndexFromItemIndex(itemIndex);
-      auto item = epub->getTocItem(tocIndex);
-      const int indentSize = 20 + bezelLeft + (item.level - 1) * 15;
-      const std::string chapterName =
-          renderer.truncatedText(UI_10_FONT_ID, item.title.c_str(), pageWidth - 40 - bezelLeft - bezelRight - indentSize + 20 + bezelLeft);
-      renderer.drawText(UI_10_FONT_ID, indentSize, 60 + bezelTop + (tocIndex % pageItems) * 30, chapterName.c_str(),
-                        tocIndex != selectorIndex);
-    }
+    // Draw TOC item
+    const int tocIndex = tocIndexFromItemIndex(itemIndex);
+    auto item = epub->getTocItem(tocIndex);
+    const int indentSize = 20 + bezelLeft + (item.level - 1) * 15;
+    const std::string chapterName =
+        renderer.truncatedText(UI_10_FONT_ID, item.title.c_str(), pageWidth - 40 - bezelLeft - bezelRight - indentSize + 20 + bezelLeft);
+    renderer.drawText(UI_10_FONT_ID, indentSize, displayY, chapterName.c_str(), !isSelected);
  }

  const auto labels = mappedInput.mapLabels("« Back", "Select", "Up", "Down");
--- a/src/activities/reader/EpubReaderChapterSelectionActivity.h
+++ b/src/activities/reader/EpubReaderChapterSelectionActivity.h
@ -30,18 +30,15 @@ class EpubReaderChapterSelectionActivity final : public ActivityWithSubactivity
  int getTotalItems() const;

  // Check if sync option is available (credentials configured)
+  // Note: Currently always returns false - placeholder for future sync feature
  bool hasSyncOption() const;

-  // Check if given item index is a sync option (first or last)
-  bool isSyncItem(int index) const;
-
  // Convert item index to TOC index (accounting for top sync option offset)
  int tocIndexFromItemIndex(int itemIndex) const;

  static void taskTrampoline(void* param);
  [[noreturn]] void displayTaskLoop();
  void renderScreen();
-  void launchSyncActivity();

 public:
  explicit EpubReaderChapterSelectionActivity(GfxRenderer& renderer, MappedInputManager& mappedInput,
--- a/src/activities/reader/TxtReaderActivity.cpp
+++ b/src/activities/reader/TxtReaderActivity.cpp
@ -88,7 +88,7 @@ void TxtReaderActivity::onEnter() {
    renderer.displayBuffer(EInkDisplay::FAST_REFRESH);

    // Generate covers with progress callback
-    txt->generateAllCovers([&](int percent) {
+    (void)txt->generateAllCovers([&](int percent) {
      const unsigned long now = millis();
      if ((now - lastUpdate) >= 3000) {
        lastUpdate = now;
@ -888,7 +888,8 @@ void TxtReaderActivity::renderEndOfBookPrompt() {
    filename = filename.substr(lastSlash + 1);
  }
  if (filename.length() > 30) {
-    filename = filename.substr(0, 27) + "...";
+    filename.resize(27);
+    filename += "...";
  }
  renderer.drawCenteredText(UI_10_FONT_ID, 120, filename.c_str());

--- a/src/main.cpp
+++ b/src/main.cpp
@ -200,35 +200,27 @@ void checkForFlashCommand() {
        constexpr int halfWidth = LOCK_ICON_WIDTH / 2;  // 16px offset for centering
        int iconX, iconY;
        GfxRenderer::ImageRotation rotation;
-        int outW, outH;  // Note: 90/270 rotation swaps output dimensions
+        // Note: 90/270 rotation swaps output dimensions (W<->H)
        switch (renderer.getOrientation()) {
          case GfxRenderer::Portrait:  // USB at bottom-left, shackle points right
            rotation = GfxRenderer::ROTATE_90;
-            outW = LOCK_ICON_HEIGHT;
-            outH = LOCK_ICON_WIDTH;
            iconX = edgeMargin;
-            iconY = screenH - outH - edgeMargin - halfWidth;
+            iconY = screenH - LOCK_ICON_WIDTH - edgeMargin - halfWidth;
            break;
          case GfxRenderer::PortraitInverted:  // USB at top-right, shackle points left
            rotation = GfxRenderer::ROTATE_270;
-            outW = LOCK_ICON_HEIGHT;
-            outH = LOCK_ICON_WIDTH;
-            iconX = screenW - outW - edgeMargin;
+            iconX = screenW - LOCK_ICON_HEIGHT - edgeMargin;
            iconY = edgeMargin + halfWidth;
            break;
          case GfxRenderer::LandscapeClockwise:  // USB at top-left, shackle points down
            rotation = GfxRenderer::ROTATE_180;
-            outW = LOCK_ICON_WIDTH;
-            outH = LOCK_ICON_HEIGHT;
            iconX = edgeMargin + halfWidth;
            iconY = edgeMargin;
            break;
          case GfxRenderer::LandscapeCounterClockwise:  // USB at bottom-right, shackle points up
            rotation = GfxRenderer::ROTATE_0;
-            outW = LOCK_ICON_WIDTH;
-            outH = LOCK_ICON_HEIGHT;
-            iconX = screenW - outW - edgeMargin - halfWidth;
-            iconY = screenH - outH - edgeMargin;
+            iconX = screenW - LOCK_ICON_WIDTH - edgeMargin - halfWidth;
+            iconY = screenH - LOCK_ICON_HEIGHT - edgeMargin;
            break;
        }
        renderer.drawImageRotated(LockIcon, iconX, iconY, LOCK_ICON_WIDTH, LOCK_ICON_HEIGHT, rotation);