diff --git a/.github/workflows/debian-forky.yml b/.github/workflows/debian-forky.yml index 1a72ea1..1b4197b 100644 --- a/.github/workflows/debian-forky.yml +++ b/.github/workflows/debian-forky.yml @@ -10,7 +10,8 @@ name: debian 14 forky jobs: - container-test-job: + container-automake: + name: automake runs-on: ubuntu-latest container: image: debian:forky-slim @@ -41,3 +42,27 @@ jobs: uses: actions/upload-artifact@v4 with: path: '**/*.log' + name: 'automake logs' + + container-cmake: + name: cmake + runs-on: ubuntu-latest + container: + image: debian:forky-slim + steps: + - name: checkout + uses: actions/checkout@v4 + - name: install packages + run: apt-get update && apt-get install cmake pkgconf build-essential nettle-dev libcap2-bin --yes + - name: configure + run: cmake -B build -S inofficial_cmake/ + - name: build + run: cmake --build build + - name: test + run: ctest --test-dir build --output-on-failure + - name: store cmake logs as artifacts + if: ${{ always() }} + uses: actions/upload-artifact@v4 + with: + path: '**/*.log' + name: 'cmake logs' diff --git a/.github/workflows/debian-trixie.yml b/.github/workflows/debian-trixie.yml index 6a31b2b..c1d1ade 100644 --- a/.github/workflows/debian-trixie.yml +++ b/.github/workflows/debian-trixie.yml @@ -10,7 +10,8 @@ name: debian 13 trixie jobs: - container-test-job: + container-automake: + name: automake runs-on: ubuntu-latest container: image: debian:trixie-slim @@ -36,8 +37,32 @@ jobs: ./configure make make check - - name: store the logs as an artifact + - name: store automake logs as artifacts if: ${{ always() }} uses: actions/upload-artifact@v4 with: path: '**/*.log' + name: 'automake logs' + + container-cmake: + name: cmake + runs-on: ubuntu-latest + container: + image: debian:trixie-slim + steps: + - name: checkout + uses: actions/checkout@v4 + - name: install packages + run: apt-get update && apt-get install cmake pkgconf build-essential nettle-dev libcap2-bin --yes + - name: configure + run: cmake -B build -S inofficial_cmake/ + - name: build + run: cmake --build build + - name: test + run: ctest --test-dir build --output-on-failure + - name: store cmake logs as artifacts + if: ${{ always() }} + uses: actions/upload-artifact@v4 + with: + path: '**/*.log' + name: 'cmake logs' diff --git a/.github/workflows/formatting.yml b/.github/workflows/formatting.yml index 06adf1b..51aecce 100644 --- a/.github/workflows/formatting.yml +++ b/.github/workflows/formatting.yml @@ -10,7 +10,7 @@ name: code formatting jobs: - build: + clang-format: name: Auto format with clang 18 runs-on: ubuntu-24.04 @@ -41,3 +41,35 @@ jobs: name: clang-format.patch path: clang-format.patch if-no-files-found: ignore + + cmake-format: + name: Auto format with cmake-format + runs-on: ubuntu-24.04 + + steps: + - name: checkout + uses: actions/checkout@v4 + - name: install packages + run: sudo apt install cmake-format + - name: run cmake format + run: | + ./do_cmake_format.sh + - name: check for differences + run: | + git diff >cmake-format.patch + if [ $(wc -c #include #include +#include #include +#include // project #include "Checksum.hh" @@ -45,6 +47,43 @@ Checksum::Checksum(checksumtypes type) } } +Checksum::Checksum(Checksum&& other) + : m_checksumtype(other.m_checksumtype) +{ +#ifdef HAVE_LIBXXHASH + if (m_checksumtype == checksumtypes::XXH128) { + m_state.xxh128 = std::exchange(other.m_state.xxh128, nullptr); + } else +#endif + { + std::memcpy(&m_state, &other.m_state, sizeof(m_state)); + } +} + +Checksum::Checksum(const Checksum& other) + : m_checksumtype(other.m_checksumtype) +{ +#ifdef HAVE_LIBXXHASH + if (m_checksumtype == checksumtypes::XXH128) { + m_state.xxh128 = XXH3_createState(); + assert(m_state.xxh128 != NULL && "Out of memory!"); + XXH3_copyState(m_state.xxh128, other.m_state.xxh128); + } else +#endif + { + std::memcpy(&m_state, &other.m_state, sizeof(m_state)); + } +} + +Checksum::~Checksum() +{ +#ifdef HAVE_LIBXXHASH + if (m_checksumtype == checksumtypes::XXH128) { + XXH3_freeState(m_state.xxh128); + } +#endif +} + int Checksum::update(std::size_t length, const unsigned char* buffer) { @@ -208,7 +247,6 @@ Checksum::printToBuffer(void* buffer, std::size_t N) XXH128_hash_t result = XXH3_128bits_digest(m_state.xxh128); XXH128_canonicalFromHash(static_cast(buffer), result); - XXH3_freeState(m_state.xxh128); } else { // bad size. return -1; diff --git a/Checksum.hh b/Checksum.hh index 247e655..83e1f85 100644 --- a/Checksum.hh +++ b/Checksum.hh @@ -38,6 +38,9 @@ public: }; explicit Checksum(checksumtypes type); + Checksum(const Checksum& other); + Checksum(Checksum&& other); + ~Checksum(); int update(std::size_t length, const unsigned char* buffer); int update(std::size_t length, const char* buffer); diff --git a/do_cmake_format.sh b/do_cmake_format.sh new file mode 100755 index 0000000..8961cfa --- /dev/null +++ b/do_cmake_format.sh @@ -0,0 +1,7 @@ +#!/bin/sh +# +# cmake autoformat. apt install cmake-format + +set -e + +cmake-format -i inofficial_cmake/CMakeLists.txt diff --git a/inofficial_cmake/CMakeLists.txt b/inofficial_cmake/CMakeLists.txt index 3a95f15..8fc7906 100644 --- a/inofficial_cmake/CMakeLists.txt +++ b/inofficial_cmake/CMakeLists.txt @@ -19,8 +19,9 @@ endif() configure_file(config.h.in config.h @ONLY) -add_executable( - rdfind +# the implementation is in this object library, to make it possible to unit test +add_library( + rdfindimpl OBJECT ../Checksum.cc ../Checksum.hh ../CmdlineParser.cc @@ -31,23 +32,32 @@ add_executable( ../EasyRandom.hh ../Fileinfo.cc ../Fileinfo.hh - ../rdfind.cc ../RdfindDebug.hh ../Rdutil.cc ../Rdutil.hh ../UndoableUnlink.cc ../UndoableUnlink.hh) -target_include_directories(rdfind PRIVATE "${CMAKE_CURRENT_BINARY_DIR}") -target_include_directories(rdfind PRIVATE ..) - -target_compile_features(rdfind PRIVATE cxx_std_17) -target_link_libraries(rdfind nettle) +target_include_directories(rdfindimpl PUBLIC "${CMAKE_CURRENT_BINARY_DIR}") +target_include_directories(rdfindimpl PUBLIC ..) +target_compile_features(rdfindimpl PUBLIC cxx_std_17) +target_compile_options(rdfindimpl PUBLIC -Wall -Wextra -Wpedantic) +target_link_libraries(rdfindimpl nettle) if(xxhash_FOUND) - target_link_libraries(rdfind PkgConfig::xxhash) + target_link_libraries(rdfindimpl PkgConfig::xxhash) endif() -target_compile_options(rdfind PRIVATE -Wall -Wextra -Wpedantic) -file(GENERATE OUTPUT .gitignore CONTENT "*") +# the executable mostly contains the main function +add_executable(rdfind ../rdfind.cc) +target_include_directories(rdfind PRIVATE ..) +target_link_libraries(rdfind PUBLIC rdfindimpl) + +file( + GENERATE + OUTPUT .gitignore + CONTENT "*") + +# apt install libcatch2-dev +find_package(Catch2 3.7) enable_testing() @@ -80,3 +90,12 @@ foreach(testscript ${testscripts}) set_tests_properties(${testname} PROPERTIES ENVIRONMENT RDFIND=$) endforeach() + +if(Catch2_FOUND) + set(unittests test_checksum) + foreach(unittest ${unittests}) + add_executable(${unittest} ../unittests/test_checksum.cc) + target_compile_features(${unittest} PRIVATE cxx_std_20) + target_link_libraries(${unittest} PRIVATE rdfindimpl Catch2::Catch2WithMain) + endforeach() +endif() diff --git a/unittests/test_checksum.cc b/unittests/test_checksum.cc new file mode 100644 index 0000000..253aeb0 --- /dev/null +++ b/unittests/test_checksum.cc @@ -0,0 +1,96 @@ +#include + +#include "Checksum.hh" +#include + +namespace { +using enum Checksum::checksumtypes; +const auto types = { MD5, + SHA1, + SHA256, + SHA512 +#ifdef HAVE_LIBXXHASH + , + XXH128 +#endif +}; + +// helper function to store the result in a string +std::string +finalize_checksum(Checksum& ck) +{ + std::string ret(static_cast(ck.getDigestLength()), ' '); + REQUIRE(0 == ck.printToBuffer(ret.data(), ret.size())); + return ret; +} + +} + +TEST_CASE("different checksums are distinct") +{ + std::set answers; + for (auto type : types) { + Checksum ck(type); + const auto answer = finalize_checksum(ck); + answers.insert(answer); + } + REQUIRE(types.size() == answers.size()); +} + +TEST_CASE("update with zero bytes is fine") +{ + for (auto type : types) { + const auto s1 = [type]() { + Checksum ck(type); + return finalize_checksum(ck); + }(); + const auto s2 = [type]() { + Checksum ck(type); + REQUIRE(0 == ck.update(0, static_cast(nullptr))); + return finalize_checksum(ck); + }(); + REQUIRE(s1 == s2); + } +} + +TEST_CASE("creating and not using it is fine") +{ + for (auto type : types) { + Checksum ck(type); + } +} + +TEST_CASE("copying a checksum is fine") +{ + static const char* content = "abcd"; + for (auto type : types) { + const auto expected = [type]() { + Checksum ck(type); + REQUIRE(0 == ck.update(std::strlen(content), content)); + return finalize_checksum(ck); + }(); + Checksum original(type); + REQUIRE(0 == original.update(std::strlen(content), content)); + Checksum copy(original); + REQUIRE(expected == finalize_checksum(copy)); + REQUIRE(expected == finalize_checksum(original)); + } +} + +TEST_CASE("copy from an rval is fine") +{ + static const char* content = "abcd"; + for (auto type : types) { + const auto expected = [type]() { + Checksum ck(type); + REQUIRE(0 == ck.update(std::strlen(content), content)); + return finalize_checksum(ck); + }(); + Checksum original(type); + REQUIRE(0 == original.update(std::strlen(content), content)); + + // move copy should be ok + Checksum movedto(std::move(original)); + REQUIRE(expected == finalize_checksum(movedto)); + } +}