Fix static analysis issues

Author: badaix

.clang-tidy

@@ -1,30 +1,11 @@
 ---
-# Checks:          '*,clang-diagnostic-*,clang-analyzer-*'
-Checks:          '*,-llvmlibc-*,-readability-braces-around-statements,-hicpp-braces-around-statements,-google-readability-braces-around-statements,-hicpp-no-array-decay,-fuchsia-default-arguments-calls,-modernize-use-trailing-return-type,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-avoid-magic-numbers,-readability-magic-numbers,-bugprone-lambda-function-name,-hicpp-signed-bitwise,-cert-err60-cpp,-cppcoreguidelines-pro-type-reinterpret-cast,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-init-variables,-cppcoreguidelines-no-malloc,-hicpp-no-malloc,-google-readability-todo,-modernize-pass-by-value,-cppcoreguidelines-pro-type-member-init,-hicpp-member-init,-hicpp-avoid-c-arrays,-cppcoreguidelines-avoid-c-arrays,-modernize-avoid-c-arrays,-abseil-string-find-startswith,-google-build-using-namespace,-cppcoreguidelines-owning-memory,-readability-else-after-return,-cppcoreguidelines-pro-type-vararg,-hicpp-vararg,-readability-isolate-declaration,-cert-env33-c,-abseil-*'
-# Checks:          '*,clang-diagnostic-*,clang-analyzer-*'
-# Checks:          '*,-llvmlibc-restrict-system-libc-headers,-readability-braces-around-statements,-hicpp-braces-around-statements,-google-readability-braces-around-statements,-hicpp-no-array-decay,-fuchsia-default-arguments-calls,-modernize-use-trailing-return-type,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-avoid-magic-numbers,-readability-magic-numbers,-bugprone-lambda-function-name,-hicpp-signed-bitwise,-cert-err60-cpp,-cppcoreguidelines-pro-type-reinterpret-cast,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-init-variables,-cppcoreguidelines-no-malloc,-hicpp-no-malloc,-google-readability-todo,-modernize-pass-by-value,-cppcoreguidelines-pro-type-member-init,-hicpp-member-init,-hicpp-avoid-c-arrays,-cppcoreguidelines-avoid-c-arrays,-modernize-avoid-c-arrays,-abseil-string-find-startswith,-google-build-using-namespace,-cppcoreguidelines-owning-memory,-readability-else-after-return,-cppcoreguidelines-pro-type-vararg,-hicpp-vararg,-readability-isolate-declaration'
+Checks:          'clang-diagnostic-*,clang-analyzer-*,modernize*,performance*,readability-*size*,readability*redundant*,-readability-redundant-member-init,-readability-redundant-access-specifiers,misc*,boost-use-to-string,cert*,google-runtime-member-string-references,google-explicit-constructor,-cert-err58-cpp,-modernize-concat-nested-namespaces,-modernize-use-nodiscard,-misc-non-private-member-variables-in-classes,-modernize-use-trailing-return-type,-misc-no-recursion,-misc-const-correctness,-misc-use-anonymous-namespace'
 WarningsAsErrors: ''
 HeaderFilterRegex: ''
 AnalyzeTemporaryDtors: false
-FormatStyle:     none
+FormatStyle:     file
 User:            johannes
 CheckOptions:
-  - key:             cert-dcl16-c.NewSuffixes
-    value:           'L;LL;LU;LLU'
-  - key:             cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField
-    value:           '0'
-  - key:             cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors
-    value:           '1'
-  - key:             cppcoreguidelines-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic
-    value:           '1'
-  - key:             google-readability-braces-around-statements.ShortStatementLines
-    value:           '1'
-  - key:             google-readability-function-size.StatementThreshold
-    value:           '800'
-  - key:             google-readability-namespace-comments.ShortNamespaceLines
-    value:           '10'
-  - key:             google-readability-namespace-comments.SpacesBeforeComments
-    value:           '2'
   - key:             modernize-loop-convert.MaxCopySize
     value:           '16'
   - key:             modernize-loop-convert.MinConfidence
@@ -37,5 +18,6 @@ CheckOptions:
     value:           llvm
   - key:             modernize-use-nullptr.NullMacros
     value:           'NULL'
+  - key:             hicpp-signed-bitwise.IgnorePositiveIntegerLiterals
+    value:           '1'
 ...
-

.clangd

@@ -0,0 +1,7 @@
+InlayHints:
+  Enabled: Yes
+  ParameterNames: No
+  DeducedTypes: No
+Diagnostics:
+  UnusedIncludes: Strict
+  MissingIncludes: None

.github/workflows/ci.yml

@@ -16,7 +16,7 @@ jobs:
         tool:
           - cppcheck
 
-    runs-on: ubuntu-22.04
+    runs-on: ubuntu-24.04
 
     steps:
       - uses: actions/checkout@v4
@@ -43,7 +43,7 @@ jobs:
       - name: configure
         run: |
           cmake -S . -B build \
-            -DWERROR=ON -DBUILD_TESTS=ON \
+            -DWERROR=ON -DBUILD_TESTS=OFF \
             -DBOOST_ROOT=boost_${BOOST_VERSION} \
             -DCMAKE_BUILD_TYPE=Release \
             -DCMAKE_EXPORT_COMPILE_COMMANDS=TRUE \

client/decoder/ogg_decoder.cpp

@@ -33,6 +33,7 @@
 #endif
 #include <cstdint>
 #include <cstring>
+#include <string>
 
 
 
@@ -43,7 +44,7 @@ static constexpr auto LOG_TAG = "OggDecoder";
 namespace decoder
 {
 
-OggDecoder::OggDecoder() : Decoder()
+OggDecoder::OggDecoder()
 {
     ogg_sync_init(&oy); /* Now we can read pages */
 }
@@ -244,7 +245,6 @@ SampleFormat OggDecoder::setHeader(msg::CodecHeader* chunk)
         if (comment.find("SAMPLE_FORMAT=") == 0)
             sampleFormat_.setFormat(comment.substr(comment.find('=') + 1));
         LOG(INFO, LOG_TAG) << "comment: " << comment << "\n";
-        ;
         ++ptr;
     }
 

client/player/alsa_player.hpp

@@ -86,7 +86,6 @@ class AlsaPlayer : public Player
     snd_pcm_uframes_t frames_;
     boost::asio::posix::stream_descriptor sd_;
     std::chrono::time_point<std::chrono::steady_clock> last_change_;
-    std::recursive_mutex mutex_;
     boost::asio::steady_timer timer_;
 
     std::optional<std::chrono::microseconds> buffer_time_;

client/player/player.hpp

@@ -117,7 +117,7 @@ class Player
     template <typename T>
     void adjustVolume(char* buffer, size_t count, double volume)
     {
-        T* bufferT = (T*)buffer;
+        auto* bufferT = static_cast<T*>(buffer);
         for (size_t n = 0; n < count; ++n)
             bufferT[n] = endian::swap<T>(static_cast<T>(endian::swap<T>(bufferT[n]) * volume));
     }

client/player/pulse_player.cpp

@@ -145,8 +145,8 @@ vector<PcmDevice> PulsePlayer::pcm_list(const std::string& parameter)
 
 
 PulsePlayer::PulsePlayer(boost::asio::io_context& io_context, const ClientSettings::Player& settings, std::shared_ptr<Stream> stream)
-    : Player(io_context, settings, stream), latency_(BUFFER_TIME), pa_ml_(nullptr), pa_ctx_(nullptr), playstream_(nullptr), proplist_(nullptr),
-      server_(std::nullopt)
+    : Player(io_context, settings, stream), latency_(BUFFER_TIME), last_chunk_tick_(0), pa_ml_(nullptr), pa_ctx_(nullptr), playstream_(nullptr),
+      proplist_(nullptr), server_(std::nullopt)
 {
     auto params = utils::string::split_pairs_to_container<std::vector<std::string>>(settings.parameter, ',', '=');
     if (params.find("buffer_time") != params.end())

cmake/cppcheck.cmake

@@ -45,7 +45,14 @@ if(CMAKE_VERSION VERSION_EQUAL "3.19.0" OR CMAKE_VERSION VERSION_GREATER
         "--suppress=*:${CMAKE_BINARY_DIR}/_deps/*"
         "--suppress=unusedFunction"
         "--suppress=noExplicitConstructor"
-        "--suppress=preprocessorErrorDirective")
+        "--suppress=missingInclude"
+        "--suppress=missingIncludeSystem"
+        "--suppress=useInitializationList"
+        "--suppress=shadowFunction"
+        "--suppress=shadowVariable"
+        "--suppress=stlIfStrFind"
+        "--suppress=useStlAlgorithm"
+        "--suppress=virtualCallInConstructor")
 
       if(CPPCHECK_ARGS)
         set(CPPCHECK_ARGS_LIST ${CPPCHECK_ARGS})

common/daemon.cpp

@@ -85,7 +85,7 @@ void Daemon::daemonize()
 
     if (!group_.empty())
     {
-        struct group* grp = getgrnam(group_.c_str());
+        const struct group* grp = getgrnam(group_.c_str());
         if (grp == nullptr)
             throw SnapException("no such group \"" + group_ + "\"");
         user_gid = grp->gr_gid;

common/error_code.hpp

@@ -126,7 +126,8 @@ struct ErrorOr
     /// @return the moved error
     ErrorCode takeError()
     {
-        return std::move(std::get<ErrorCode>(var));
+        auto ec = std::move(std::get<ErrorCode>(var));
+        return ec;
     }
 
 private:

common/message/codec_header.hpp

@@ -31,11 +31,11 @@ namespace msg
 class CodecHeader : public BaseMessage
 {
 public:
-    CodecHeader(const std::string& codecName = "", uint32_t size = 0)
+    explicit CodecHeader(const std::string& codecName = "", uint32_t size = 0)
         : BaseMessage(message_type::kCodecHeader), payloadSize(size), payload(nullptr), codec(codecName)
     {
         if (size > 0)
-            payload = (char*)malloc(size * sizeof(char));
+            payload = static_cast<char*>(malloc(size * sizeof(char)));
     }
 
     ~CodecHeader() override

common/message/message.hpp

@@ -116,7 +116,7 @@ struct tv
     }
 
     /// C'tor, construct from timeval @p tv
-    tv(timeval tv) : sec(tv.tv_sec), usec(tv.tv_usec){};
+    explicit tv(timeval tv) : sec(tv.tv_sec), usec(tv.tv_usec){};
     /// C'tor, construct from @p _sec and @p _usec
     tv(int32_t _sec, int32_t _usec) : sec(_sec), usec(_usec){};
 
@@ -173,7 +173,7 @@ struct BaseMessage
     }
 
     /// c'tor with message type @p type_
-    BaseMessage(message_type type_) : type(type_), id(0), refersTo(0), size(0)
+    explicit BaseMessage(message_type type_) : type(type_), id(0), refersTo(0), size(0)
     {
     }
 
@@ -370,7 +370,7 @@ struct BaseMessage
     void readVal(std::istream& stream, char** payload, uint32_t& size) const
     {
         readVal(stream, size);
-        *payload = (char*)realloc(*payload, size);
+        *payload = static_cast<char*>(realloc(*payload, size));
         stream.read(*payload, size);
     }
 

common/message/pcm_chunk.hpp

@@ -1,6 +1,6 @@
 /***
     This file is part of snapcast
-    Copyright (C) 2014-2022  Johannes Pohl
+    Copyright (C) 2014-2024  Johannes Pohl
 
     This program is free software: you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -83,7 +83,7 @@ class PcmChunk : public WireChunk
 
         // logd << ", from: " << format.frameSize()*idx << ", to: " << format.frameSize()*idx + format.frameSize()*result;
         if (outputBuffer != nullptr)
-            memcpy((char*)outputBuffer, (char*)(payload) + format.frameSize() * idx_, format.frameSize() * result);
+            memcpy(static_cast<char*>(outputBuffer), static_cast<char*>(payload) + format.frameSize() * idx_, format.frameSize() * result);
 
         idx_ += result;
         // logd << ", new idx: " << idx << ", result: " << result << ", wireChunk->length: " << wireChunk->length << ", format.frameSize(): " <<

common/message/wire_chunk.hpp

@@ -39,15 +39,15 @@ namespace msg
 class WireChunk : public BaseMessage
 {
 public:
-    WireChunk(uint32_t size = 0) : BaseMessage(message_type::kWireChunk), payloadSize(size), payload(nullptr)
+    explicit WireChunk(uint32_t size = 0) : BaseMessage(message_type::kWireChunk), payloadSize(size), payload(nullptr)
     {
         if (size > 0)
-            payload = (char*)malloc(size * sizeof(char));
+            payload = static_cast<char*>(malloc(size * sizeof(char)));
     }
 
     WireChunk(const WireChunk& wireChunk) : BaseMessage(message_type::kWireChunk), timestamp(wireChunk.timestamp), payloadSize(wireChunk.payloadSize)
     {
-        payload = (char*)malloc(payloadSize);
+        payload = static_cast<char*>(malloc(payloadSize));
         memcpy(payload, wireChunk.payload, payloadSize);
     }
 

common/utils/file_utils.hpp

@@ -33,7 +33,6 @@
 #endif
 
 #include <filesystem>
-#include <fstream>
 #include <stdexcept>
 #include <vector>
 
@@ -64,15 +63,15 @@ static void do_chown(const std::string& file_path, const std::string& user_name,
 
     if (!user_name.empty())
     {
-        struct passwd* pwd = getpwnam(user_name.c_str());
+        const struct passwd* pwd = getpwnam(user_name.c_str());
         if (pwd == nullptr)
             throw std::runtime_error("Failed to get uid");
         uid = pwd->pw_uid;
     }
 
     if (!group_name.empty())
     {
-        struct group* grp = getgrnam(group_name.c_str());
+        const struct group* grp = getgrnam(group_name.c_str());
         if (grp == nullptr)
             throw std::runtime_error("Failed to get gid");
         gid = grp->gr_gid;

common/utils/string_utils.hpp

@@ -75,7 +75,7 @@ std::map<std::string, T> split_pairs_to_container(const std::string& s, char pai
 {
     std::map<std::string, T> result;
     auto keyValueList = split(s, pair_delim);
-    for (auto& kv : keyValueList)
+    for (const auto& kv : keyValueList)
     {
         auto pos = kv.find(key_value_delim);
         if (pos != std::string::npos)

server/control_session.hpp

@@ -27,7 +27,6 @@
 // standard headers
 #include <functional>
 #include <memory>
-#include <optional>
 #include <string>
 
 

server/control_session_ws.cpp

@@ -1,6 +1,6 @@
 /***
     This file is part of snapcast
-    Copyright (C) 2014-2023  Johannes Pohl
+    Copyright (C) 2014-2024  Johannes Pohl
 
     This program is free software: you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -74,7 +74,7 @@ void ControlSessionWebsocket::stop()
 void ControlSessionWebsocket::sendAsync(const std::string& message)
 {
     boost::asio::post(strand_,
-                      [this, self = shared_from_this(), msg = message]()
+                      [this, self = shared_from_this(), msg = message]() mutable
                       {
         messages_.push_back(std::move(msg));
         if (messages_.size() > 1)
@@ -147,7 +147,7 @@ void ControlSessionWebsocket::on_read_ws(beast::error_code ec, std::size_t bytes
     if (!line.empty())
     {
         // LOG(DEBUG, LOG_TAG) << "received: " << line << "\n";
-        if ((message_receiver_ != nullptr) && !line.empty())
+        if (message_receiver_ != nullptr)
         {
             message_receiver_->onMessageReceived(shared_from_this(), line,
                                                  [this](const std::string& response)

server/encoder/flac_encoder.hpp

@@ -25,17 +25,17 @@
 #include "FLAC/stream_encoder.h"
 
 // standard headers
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
 
 namespace encoder
 {
 
 class FlacEncoder : public Encoder
 {
 public:
-    FlacEncoder(const std::string& codecOptions = "");
+    explicit FlacEncoder(const std::string& codecOptions = "");
     ~FlacEncoder() override;
     void encode(const msg::PcmChunk& chunk) override;
     std::string getAvailableOptions() const override;

server/encoder/ogg_encoder.cpp

@@ -84,19 +84,19 @@ void OggEncoder::encode(const msg::PcmChunk& chunk)
     {
         if (sampleFormat_.sampleSize() == 1)
         {
-            auto* chunkBuffer = reinterpret_cast<int8_t*>(chunk.payload);
+            const auto* chunkBuffer = reinterpret_cast<int8_t*>(chunk.payload);
             for (int i = 0; i < frames; i++)
                 buffer[channel][i] = chunkBuffer[sampleFormat_.channels() * i + channel] / 128.f;
         }
         else if (sampleFormat_.sampleSize() == 2)
         {
-            auto* chunkBuffer = reinterpret_cast<int16_t*>(chunk.payload);
+            const auto* chunkBuffer = reinterpret_cast<int16_t*>(chunk.payload);
             for (int i = 0; i < frames; i++)
                 buffer[channel][i] = chunkBuffer[sampleFormat_.channels() * i + channel] / 32768.f;
         }
         else if (sampleFormat_.sampleSize() == 4)
         {
-            auto* chunkBuffer = reinterpret_cast<int32_t*>(chunk.payload);
+            const auto* chunkBuffer = reinterpret_cast<int32_t*>(chunk.payload);
             for (int i = 0; i < frames; i++)
                 buffer[channel][i] = chunkBuffer[sampleFormat_.channels() * i + channel] / 2147483648.f;
         }

server/encoder/opus_encoder.cpp

@@ -44,7 +44,7 @@ namespace
 template <typename T>
 void assign(void* pointer, T val)
 {
-    T* p = (T*)pointer;
+    T* p = static_cast<T*>(pointer);
     *p = val;
 }
 } // namespace
@@ -246,7 +246,7 @@ void OpusEncoder::encode(const SampleFormat& format, const char* data, size_t si
     if (encoded_.size() < size)
         encoded_.resize(size);
 
-    opus_int32 len = opus_encode(enc_, (opus_int16*)data, samples_per_channel, encoded_.data(), size);
+    opus_int32 len = opus_encode(enc_, reinterpret_cast<const opus_int16*>(data), samples_per_channel, encoded_.data(), size);
     LOG(TRACE, LOG_TAG) << "Encode " << samples_per_channel << " frames, size " << size << " bytes, encoded: " << len << " bytes" << '\n';
 
     if (len > 0)

server/encoder/pcm_encoder.cpp

@@ -1,6 +1,6 @@
 /***
     This file is part of snapcast
-    Copyright (C) 2014-2021  Johannes Pohl
+    Copyright (C) 2014-2024  Johannes Pohl
 
     This program is free software: you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -42,7 +42,7 @@ namespace
 template <typename T>
 void assign(void* pointer, T val)
 {
-    T* p = (T*)pointer;
+    T* p = static_cast<T*>(pointer);
     *p = val;
 }
 } // namespace