From e0f40b02ea579b8c31296b55aa643ef4805c013f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Kubern=C3=A1t?= Date: Fri, 26 Jan 2024 19:55:07 +0100 Subject: [PATCH 1/7] Do not use c-style arrays --- libnecrolog/necrolog.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libnecrolog/necrolog.cpp b/libnecrolog/necrolog.cpp index a90369a..22fc6a6 100644 --- a/libnecrolog/necrolog.cpp +++ b/libnecrolog/necrolog.cpp @@ -140,6 +140,7 @@ static void parse_thresholds_string(const std::string &thresholds, std::map NecroLog::setCLIOptions(int argc, char *argv[]) { std::vector params; @@ -423,9 +424,9 @@ void NecroLog::writeWithDefaultFormat(std::ostream &os, bool is_colorized, Necro { std::time_t t = std::time(nullptr); std::tm *tm = std::gmtime(&t); /// gmtime is not thread safe!!! - char buffer[80] = {0}; - std::strftime(buffer, sizeof(buffer),"%Y-%m-%dT%H:%M:%S", tm); - set_TTY_color(is_colorized, os, TTYColor::Green) << std::string(buffer); + std::array buffer = {0}; + std::strftime(buffer.data(), buffer.size() * sizeof(*buffer.data()),"%Y-%m-%dT%H:%M:%S", tm); + set_TTY_color(is_colorized, os, TTYColor::Green) << std::string(buffer.data()); set_TTY_color(is_colorized, os, TTYColor::Brown) << '[' << moduleFromFileName(context.file()) << ':' << context.line() << "]"; if(context.isTopicSet()) { set_TTY_color(is_colorized, os, TTYColor::White) << '(' << context.topic() << ")"; From 37724a83eaab6b74b706d3794cb41f41c2596fa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Kubern=C3=A1t?= Date: Fri, 26 Jan 2024 19:55:15 +0100 Subject: [PATCH 2/7] Prefer emplace_back --- libnecrolog/necrolog.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnecrolog/necrolog.cpp b/libnecrolog/necrolog.cpp index 22fc6a6..612d58a 100644 --- a/libnecrolog/necrolog.cpp +++ b/libnecrolog/necrolog.cpp @@ -145,7 +145,7 @@ std::vector NecroLog::setCLIOptions(int argc, char *argv[]) { std::vector params; for (int i = 0; i < argc; ++i) - params.push_back(argv[i]); + params.emplace_back(argv[i]); return setCLIOptions(params); } From 9272ce0c81c4e37e4fe6252f761dcec1c8e99433 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Kubern=C3=A1t?= Date: Fri, 26 Jan 2024 20:00:27 +0100 Subject: [PATCH 3/7] Do not use else after return --- libnecrolog/necrolog.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libnecrolog/necrolog.cpp b/libnecrolog/necrolog.cpp index 612d58a..8740935 100644 --- a/libnecrolog/necrolog.cpp +++ b/libnecrolog/necrolog.cpp @@ -91,9 +91,7 @@ bool NecroLog::shouldLog(Level level, const LogContext &context) if(topic_set) { return level <= Level::Warning; } - else { - return level <= Level::Info; // log non-topic INFO messages - } + return level <= Level::Info; // log non-topic INFO messages } return false; } From 96fcda48340aa5b07f473875249c30535194b71a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Kubern=C3=A1t?= Date: Fri, 26 Jan 2024 20:07:37 +0100 Subject: [PATCH 4/7] Use anonymous namespaces --- libnecrolog/necrolog.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libnecrolog/necrolog.cpp b/libnecrolog/necrolog.cpp index 8740935..b54db1b 100644 --- a/libnecrolog/necrolog.cpp +++ b/libnecrolog/necrolog.cpp @@ -103,7 +103,8 @@ NecroLog::MessageHandler NecroLog::setMessageHandler(NecroLog::MessageHandler h) return ret; } -static void parse_thresholds_string(const std::string &thresholds, std::map &threshold_map) +namespace { +void parse_thresholds_string(const std::string &thresholds, std::map &threshold_map) { using namespace std; threshold_map.clear(); @@ -137,6 +138,7 @@ static void parse_thresholds_string(const std::string &thresholds, std::map NecroLog::setCLIOptions(int argc, char *argv[]) @@ -220,7 +222,8 @@ NecroLog::Level NecroLog::stringToLevel(const char *str) return NecroLog::Level::Invalid; } -static std::string levels_to_string(const std::map &thresholds) +namespace { +std::string levels_to_string(const std::map &thresholds) { std::string ret; for (auto& kv : thresholds) { @@ -232,6 +235,7 @@ static std::string levels_to_string(const std::map } return ret; } +} std::string NecroLog::thresholdsLogInfo() { From 1bd8974ce8d6f074b5bf18d1e5f95902009bcdec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Kubern=C3=A1t?= Date: Fri, 26 Jan 2024 20:37:37 +0100 Subject: [PATCH 5/7] Don't move trivially-copyable structs It does nothing. >:( --- libnecrolog/necrolog.cpp | 8 ++++---- libnecrolog/necrolog.h | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/libnecrolog/necrolog.cpp b/libnecrolog/necrolog.cpp index b54db1b..462d056 100644 --- a/libnecrolog/necrolog.cpp +++ b/libnecrolog/necrolog.cpp @@ -15,9 +15,9 @@ NecroLog::Options &NecroLog::globalOptions() return global_options; } -NecroLog NecroLog::create(Level level, LogContext &&log_context) +NecroLog NecroLog::create(Level level, const LogContext& log_context) { - return NecroLog(level, std::move(log_context)); + return NecroLog(level, log_context); } bool NecroLog::shouldLog(Level level, const LogContext &context) @@ -318,9 +318,9 @@ const char * NecroLog::cliHelp() return ret; } -NecroLog::NecroLogSharedData::NecroLogSharedData(NecroLog::Level level, NecroLog::LogContext &&log_context) +NecroLog::NecroLogSharedData::NecroLogSharedData(NecroLog::Level level, const NecroLog::LogContext& log_context) : m_level(level) - , m_logContext(std::move(log_context)) + , m_logContext(log_context) { } diff --git a/libnecrolog/necrolog.h b/libnecrolog/necrolog.h index 4de38c4..312a770 100644 --- a/libnecrolog/necrolog.h +++ b/libnecrolog/necrolog.h @@ -45,10 +45,10 @@ class NECROLOG_DECL_EXPORT NecroLog using MessageHandler = std::function; public: - NecroLog(Level level, LogContext &&log_context) + NecroLog(Level level, const LogContext& log_context) { if(level > Level::Invalid) - m_necro = std::make_shared(level, std::move(log_context)); + m_necro = std::make_shared(level, log_context); } template @@ -57,7 +57,7 @@ class NECROLOG_DECL_EXPORT NecroLog NecroLog& nospace() {if(m_necro) m_necro->setSpace(false); return *this;} NecroLog& color(NecroLog::Color c) {if(m_necro) m_necro->setColor(c); return *this;} - static NecroLog create(Level level, LogContext &&log_context); + static NecroLog create(Level level, const LogContext &log_context); static bool shouldLog(Level level, const LogContext &context); static MessageHandler setMessageHandler(MessageHandler h); @@ -101,7 +101,7 @@ class NECROLOG_DECL_EXPORT NecroLog class NECROLOG_DECL_EXPORT NecroLogSharedData { friend class NecroLog; public: - NecroLogSharedData(NecroLog::Level level, LogContext &&log_context); + NecroLogSharedData(NecroLog::Level level, const LogContext& log_context); ~NecroLogSharedData(); void setSpace(bool b) {m_isSpace = b;} From c1a7dde36bd39dc102d80df9def72ce3e7d39f52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Kubern=C3=A1t?= Date: Fri, 26 Jan 2024 20:44:02 +0100 Subject: [PATCH 6/7] Init string to nullptr --- libnecrolog/necrolog.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnecrolog/necrolog.cpp b/libnecrolog/necrolog.cpp index 462d056..bb43376 100644 --- a/libnecrolog/necrolog.cpp +++ b/libnecrolog/necrolog.cpp @@ -31,7 +31,7 @@ bool NecroLog::shouldLog(Level level, const LogContext &context) //if(!topic_set && opts.fileThresholds.empty()) // return level <= Level::Info; // when thresholds are not set, log non-topic INFO messages - const char *searched_str = ""; + const char *searched_str = nullptr; if(topic_set) { searched_str = context.topic(); } From 0bb4aeda2b21e2352cd0ab77b16977e1a834658e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Kubern=C3=A1t?= Date: Fri, 26 Jan 2024 21:21:00 +0100 Subject: [PATCH 7/7] Add CI --- .github/actions/build-and-test/action.yml | 17 +++++++ .github/actions/cmake/action.yml | 45 +++++++++++++++++ .github/actions/run-linter/action.yml | 24 +++++++++ .github/workflows/lint.yml | 41 ++++++++++++++++ .github/workflows/test.yml | 59 +++++++++++++++++++++++ 5 files changed, 186 insertions(+) create mode 100644 .github/actions/build-and-test/action.yml create mode 100644 .github/actions/cmake/action.yml create mode 100644 .github/actions/run-linter/action.yml create mode 100644 .github/workflows/lint.yml create mode 100644 .github/workflows/test.yml diff --git a/.github/actions/build-and-test/action.yml b/.github/actions/build-and-test/action.yml new file mode 100644 index 0000000..196a352 --- /dev/null +++ b/.github/actions/build-and-test/action.yml @@ -0,0 +1,17 @@ +name: Build and test +description: "Build the project and run tests via CMake" + +runs: + using: "composite" + steps: + - name: Build + run: cmake --build '${{github.workspace}}/build' --parallel "$(nproc)" + shell: bash + + - name: Test + working-directory: '${{github.workspace}}/build' + run: | + # https://github.com/DiligentGraphics/github-action/commit/d2f7990b16def1efa337e5cc2fc8fa22b6fae55d + PATH="/c/mingw64/bin:$PATH" + ctest --output-on-failure -j"$(nproc)" + shell: bash diff --git a/.github/actions/cmake/action.yml b/.github/actions/cmake/action.yml new file mode 100644 index 0000000..68eb1ec --- /dev/null +++ b/.github/actions/cmake/action.yml @@ -0,0 +1,45 @@ +name: Setup CMake +description: "Invoke CMake and generate build files" +inputs: + additional_cmake_args: + description: "Additional args to pass to CMake" + default: "" + +runs: + using: "composite" + steps: + # Linux deps + - name: Install/cache clazy, and doctest + if: runner.os != 'Windows' + uses: awalsh128/cache-apt-pkgs-action@v1.3.0 + with: + packages: doctest-dev clazy + version: 1.0 + + # Windows deps + - name: Install Windows deps + if: runner.os == 'Windows' + run: | + vcpkg install doctest:x64-mingw-dynamic + echo cmake_extra_args="'-DCMAKE_TOOLCHAIN_FILE=$VCPKG_INSTALLATION_ROOT/scripts/buildsystems/vcpkg.cmake' '-DMINGW=ON' '-G MinGW Makefiles'" >> "$GITHUB_ENV" + shell: bash + + - name: ccache + uses: hendrikmuhs/ccache-action@v1.2 + with: + key: ${{ github.job }} + + - name: Configure CMake + run: | + CFLAGS="-Werror ${CFLAGS}" \ + CXXFLAGS="-Werror ${CXXFLAGS}" \ + cmake \ + -S '${{github.workspace}}' \ + -B '${{github.workspace}}/build' \ + -DCMAKE_BUILD_TYPE=Release \ + -DCMAKE_C_COMPILER_LAUNCHER=ccache \ + -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \ + -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ + ${{ env.cmake_extra_args }} \ + ${{ inputs.additional_cmake_args }} + shell: bash diff --git a/.github/actions/run-linter/action.yml b/.github/actions/run-linter/action.yml new file mode 100644 index 0000000..d22425e --- /dev/null +++ b/.github/actions/run-linter/action.yml @@ -0,0 +1,24 @@ +name: run-linter +description: "Run a linter on all changed files" +inputs: + lint_program_with_args: + description: "Which program to run" + required: true + +runs: + using: "composite" + steps: + - uses: ./.github/actions/cmake + + - uses: mjp41/workaround8649@c8550b715ccdc17f89c8d5c28d7a48eeff9c94a8 + if: runner.os == 'Linux' + with: + os: ubuntu-latest + + - name: Copy compile_commands.json + shell: bash + run: cp build/compile_commands.json compile_commands.json + + - name: Run the linter + shell: bash + run: find -name '*.cpp' | parallel --verbose --jobs "$(nproc)" --plus _=[{#}/{##}] ${{ inputs.lint_program_with_args }} {} diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 0000000..4349c5a --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,41 @@ +# vim: sw=2 +name: Lint + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +on: + push: + branches: [ "master" ] + pull_request: + branches: [ "master" ] + +jobs: + clang-tidy: + name: clang-tidy / Ubuntu 22.04 + runs-on: ubuntu-22.04 + env: + CC: clang-15 + CXX: clang++-15 + steps: + - uses: actions/checkout@v3 + + - name: Run clang-tidy + uses: ./.github/actions/run-linter + with: + lint_program_with_args: clang-tidy-15 --quiet --warnings-as-errors=* + + clazy: + name: clazy / Ubuntu 22.04 + runs-on: ubuntu-22.04 + env: + CC: clang-15 + CXX: clang++-15 + steps: + - uses: actions/checkout@v3 + + - name: Run clazy + uses: ./.github/actions/run-linter + with: + lint_program_with_args: clazy-standalone --checks=level1,no-fully-qualified-moc-types,no-non-pod-global-static,no-range-loop-detach diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..814f2e8 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,59 @@ +# vim: sw=2 +name: Test + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +on: + push: + branches: [ "master" ] + pull_request: + branches: [ "master" ] + +jobs: + ubuntu-qt5: + name: Qt 5.15.2 / Ubuntu 22.04 + runs-on: ubuntu-22.04 + env: + CFLAGS: -fsanitize=address,undefined + CXXFLAGS: -fsanitize=address,undefined + steps: + - uses: actions/checkout@v3 + + - name: Setup CMake + uses: ./.github/actions/cmake + + - name: Build and test + uses: ./.github/actions/build-and-test + + ubuntu-qt6: + name: Qt 6.5.0 / Ubuntu 22.04 + runs-on: ubuntu-22.04 + env: + CFLAGS: -fsanitize=address,undefined + CXXFLAGS: -fsanitize=address,undefined + steps: + - uses: actions/checkout@v3 + + - name: Setup CMake + uses: ./.github/actions/cmake + + - name: Build and test + uses: ./.github/actions/build-and-test + + windows: + name: Qt 6.5.0 / Windows + runs-on: windows-2022 + steps: + - uses: actions/checkout@v3 + + - name: Setup CMake + uses: ./.github/actions/cmake + with: + additional_cmake_args: | + -DCMAKE_RUNTIME_OUTPUT_DIRECTORY='${{github.workspace}}/build/bin' \ + -DCMAKE_LIBRARY_OUTPUT_DIRECTORY='${{github.workspace}}/build/bin' + + - name: Build and test + uses: ./.github/actions/build-and-test