From 17efc6799ca6bc4f88f055b1a5bdd8f3bd812627 Mon Sep 17 00:00:00 2001 From: badaix Date: Fri, 20 Dec 2024 21:56:12 +0100 Subject: [PATCH] Fix static analysis issues --- .clang-tidy | 26 ++++---------------------- .clangd | 7 +++++++ .github/workflows/ci.yml | 4 ++-- client/decoder/ogg_decoder.cpp | 4 ++-- client/player/alsa_player.hpp | 1 - client/player/player.hpp | 2 +- client/player/pulse_player.cpp | 4 ++-- cmake/cppcheck.cmake | 9 ++++++++- common/daemon.cpp | 2 +- common/error_code.hpp | 3 ++- common/message/codec_header.hpp | 4 ++-- common/message/message.hpp | 6 +++--- common/message/pcm_chunk.hpp | 4 ++-- common/message/wire_chunk.hpp | 6 +++--- common/utils/file_utils.hpp | 5 ++--- common/utils/string_utils.hpp | 2 +- server/control_session.hpp | 1 - server/control_session_ws.cpp | 6 +++--- server/encoder/flac_encoder.hpp | 8 ++++---- server/encoder/ogg_encoder.cpp | 6 +++--- server/encoder/opus_encoder.cpp | 4 ++-- server/encoder/pcm_encoder.cpp | 4 ++-- 22 files changed, 56 insertions(+), 62 deletions(-) create mode 100644 .clangd diff --git a/.clang-tidy b/.clang-tidy index ca301896..3962720f 100644 --- a/.clang-tidy +++ b/.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' ... - diff --git a/.clangd b/.clangd new file mode 100644 index 00000000..d24233a1 --- /dev/null +++ b/.clangd @@ -0,0 +1,7 @@ +InlayHints: + Enabled: Yes + ParameterNames: No + DeducedTypes: No +Diagnostics: + UnusedIncludes: Strict + MissingIncludes: None diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 90fadd5e..61c10f30 100644 --- a/.github/workflows/ci.yml +++ b/.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 \ diff --git a/client/decoder/ogg_decoder.cpp b/client/decoder/ogg_decoder.cpp index 5459f4a8..be491be1 100644 --- a/client/decoder/ogg_decoder.cpp +++ b/client/decoder/ogg_decoder.cpp @@ -33,6 +33,7 @@ #endif #include #include +#include @@ -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; } diff --git a/client/player/alsa_player.hpp b/client/player/alsa_player.hpp index d189bc29..fda6209a 100644 --- a/client/player/alsa_player.hpp +++ b/client/player/alsa_player.hpp @@ -86,7 +86,6 @@ private: snd_pcm_uframes_t frames_; boost::asio::posix::stream_descriptor sd_; std::chrono::time_point last_change_; - std::recursive_mutex mutex_; boost::asio::steady_timer timer_; std::optional buffer_time_; diff --git a/client/player/player.hpp b/client/player/player.hpp index e372402f..f20dffd6 100644 --- a/client/player/player.hpp +++ b/client/player/player.hpp @@ -117,7 +117,7 @@ private: template void adjustVolume(char* buffer, size_t count, double volume) { - T* bufferT = (T*)buffer; + auto* bufferT = static_cast(buffer); for (size_t n = 0; n < count; ++n) bufferT[n] = endian::swap(static_cast(endian::swap(bufferT[n]) * volume)); } diff --git a/client/player/pulse_player.cpp b/client/player/pulse_player.cpp index ee4e8f70..6c578457 100644 --- a/client/player/pulse_player.cpp +++ b/client/player/pulse_player.cpp @@ -145,8 +145,8 @@ vector PulsePlayer::pcm_list(const std::string& parameter) PulsePlayer::PulsePlayer(boost::asio::io_context& io_context, const ClientSettings::Player& settings, std::shared_ptr 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>(settings.parameter, ',', '='); if (params.find("buffer_time") != params.end()) diff --git a/cmake/cppcheck.cmake b/cmake/cppcheck.cmake index a8e85e27..ea48dc51 100644 --- a/cmake/cppcheck.cmake +++ b/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}) diff --git a/common/daemon.cpp b/common/daemon.cpp index aa997567..0884e2ba 100644 --- a/common/daemon.cpp +++ b/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; diff --git a/common/error_code.hpp b/common/error_code.hpp index f8bde035..4050e7b1 100644 --- a/common/error_code.hpp +++ b/common/error_code.hpp @@ -126,7 +126,8 @@ struct ErrorOr /// @return the moved error ErrorCode takeError() { - return std::move(std::get(var)); + auto ec = std::move(std::get(var)); + return ec; } private: diff --git a/common/message/codec_header.hpp b/common/message/codec_header.hpp index cf34eb5e..3f91c971 100644 --- a/common/message/codec_header.hpp +++ b/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(malloc(size * sizeof(char))); } ~CodecHeader() override diff --git a/common/message/message.hpp b/common/message/message.hpp index 3b534a30..383b0374 100644 --- a/common/message/message.hpp +++ b/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 @@ protected: void readVal(std::istream& stream, char** payload, uint32_t& size) const { readVal(stream, size); - *payload = (char*)realloc(*payload, size); + *payload = static_cast(realloc(*payload, size)); stream.read(*payload, size); } diff --git a/common/message/pcm_chunk.hpp b/common/message/pcm_chunk.hpp index ce5fd3b0..be86f3bf 100644 --- a/common/message/pcm_chunk.hpp +++ b/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 @@ public: // 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(outputBuffer), static_cast(payload) + format.frameSize() * idx_, format.frameSize() * result); idx_ += result; // logd << ", new idx: " << idx << ", result: " << result << ", wireChunk->length: " << wireChunk->length << ", format.frameSize(): " << diff --git a/common/message/wire_chunk.hpp b/common/message/wire_chunk.hpp index 979175e9..932bbf23 100644 --- a/common/message/wire_chunk.hpp +++ b/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(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(malloc(payloadSize)); memcpy(payload, wireChunk.payload, payloadSize); } diff --git a/common/utils/file_utils.hpp b/common/utils/file_utils.hpp index 00f0853a..fac4e560 100644 --- a/common/utils/file_utils.hpp +++ b/common/utils/file_utils.hpp @@ -33,7 +33,6 @@ #endif #include -#include #include #include @@ -64,7 +63,7 @@ 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; @@ -72,7 +71,7 @@ static void do_chown(const std::string& file_path, const std::string& user_name, 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; diff --git a/common/utils/string_utils.hpp b/common/utils/string_utils.hpp index 740b0662..81e2a947 100644 --- a/common/utils/string_utils.hpp +++ b/common/utils/string_utils.hpp @@ -75,7 +75,7 @@ std::map split_pairs_to_container(const std::string& s, char pai { std::map 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) diff --git a/server/control_session.hpp b/server/control_session.hpp index 5d02957c..91b126d0 100644 --- a/server/control_session.hpp +++ b/server/control_session.hpp @@ -27,7 +27,6 @@ // standard headers #include #include -#include #include diff --git a/server/control_session_ws.cpp b/server/control_session_ws.cpp index 256564fc..d2ce4226 100644 --- a/server/control_session_ws.cpp +++ b/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) diff --git a/server/encoder/flac_encoder.hpp b/server/encoder/flac_encoder.hpp index 2a9a5a23..66d06e3e 100644 --- a/server/encoder/flac_encoder.hpp +++ b/server/encoder/flac_encoder.hpp @@ -25,9 +25,9 @@ #include "FLAC/stream_encoder.h" // standard headers -#include -#include -#include +#include +#include +#include namespace encoder { @@ -35,7 +35,7 @@ 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; diff --git a/server/encoder/ogg_encoder.cpp b/server/encoder/ogg_encoder.cpp index 5df32886..f013c587 100644 --- a/server/encoder/ogg_encoder.cpp +++ b/server/encoder/ogg_encoder.cpp @@ -84,19 +84,19 @@ void OggEncoder::encode(const msg::PcmChunk& chunk) { if (sampleFormat_.sampleSize() == 1) { - auto* chunkBuffer = reinterpret_cast(chunk.payload); + const auto* chunkBuffer = reinterpret_cast(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(chunk.payload); + const auto* chunkBuffer = reinterpret_cast(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(chunk.payload); + const auto* chunkBuffer = reinterpret_cast(chunk.payload); for (int i = 0; i < frames; i++) buffer[channel][i] = chunkBuffer[sampleFormat_.channels() * i + channel] / 2147483648.f; } diff --git a/server/encoder/opus_encoder.cpp b/server/encoder/opus_encoder.cpp index 9a97fb81..d9423de3 100644 --- a/server/encoder/opus_encoder.cpp +++ b/server/encoder/opus_encoder.cpp @@ -44,7 +44,7 @@ namespace template void assign(void* pointer, T val) { - T* p = (T*)pointer; + T* p = static_cast(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(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) diff --git a/server/encoder/pcm_encoder.cpp b/server/encoder/pcm_encoder.cpp index 676dd585..27fe0cac 100644 --- a/server/encoder/pcm_encoder.cpp +++ b/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 void assign(void* pointer, T val) { - T* p = (T*)pointer; + T* p = static_cast(pointer); *p = val; } } // namespace