From 3d9f5efc26b91d7c9bf9104f55551ea7ad454dc2 Mon Sep 17 00:00:00 2001 From: Sergei Date: Fri, 16 Aug 2024 17:15:27 +0300 Subject: [PATCH] Don't call `memory.deallocate` for `NULL` memory (#59) Also: - Latest `toolshed:ts22.4.3` -> `22.4.10` : gcc 11.3 -> 12.3 - Disable some new clang-tidies --- .clang-tidy | 4 ++++ .github/workflows/main.yml | 8 ++++---- libudpard/udpard.c | 24 ++++++++++++++++-------- tests/.clang-tidy | 3 +++ 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index c711162..808185c 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -22,6 +22,10 @@ Checks: >- -hicpp-static-assert, -misc-static-assert, -modernize-macro-to-enum, + -cppcoreguidelines-macro-to-enum, + -bugprone-casting-through-void, + -misc-include-cleaner, + -cppcoreguidelines-avoid-do-while, CheckOptions: - key: readability-function-cognitive-complexity.Threshold value: '99' diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index fc862ed..fe4c30c 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -6,7 +6,7 @@ jobs: debug: if: github.event_name == 'push' runs-on: ubuntu-latest - container: ghcr.io/opencyphal/toolshed:ts22.4.3 + container: ghcr.io/opencyphal/toolshed:ts22.4.10 strategy: matrix: toolchain: [ 'clang', 'gcc' ] @@ -46,7 +46,7 @@ jobs: optimizations: if: github.event_name == 'push' runs-on: ubuntu-latest - container: ghcr.io/opencyphal/toolshed:ts22.4.3 + container: ghcr.io/opencyphal/toolshed:ts22.4.10 strategy: matrix: toolchain: [ 'clang', 'gcc' ] @@ -55,7 +55,7 @@ jobs: - toolchain: gcc c-compiler: gcc cxx-compiler: g++ - cxx-flags: -fno-strict-aliasing # GCC in MinSizeRel C++20 mode misoptimizes the Cavl test. + cxx-flags: -fno-strict-aliasing # GCC in MinSizeRel C++20 mode misoptimizes the Cavl test. - toolchain: clang c-compiler: clang cxx-compiler: clang++ @@ -123,7 +123,7 @@ jobs: sonar: runs-on: ubuntu-latest - container: ghcr.io/opencyphal/toolshed:ts22.4.3 + container: ghcr.io/opencyphal/toolshed:ts22.4.10 if: > ( (github.event_name == 'pull_request' || contains(github.ref, '/main') || contains(github.ref, '/release')) && diff --git a/libudpard/udpard.c b/libudpard/udpard.c index 10a4d36..9ff828b 100644 --- a/libudpard/udpard.c +++ b/libudpard/udpard.c @@ -140,7 +140,10 @@ static inline void memFree(const struct UdpardMemoryResource memory, const size_ static inline void memFreePayload(const struct UdpardMemoryDeleter memory, const struct UdpardMutablePayload payload) { UDPARD_ASSERT(memory.deallocate != NULL); - memory.deallocate(memory.user_reference, payload.size, payload.data); + if (payload.data != NULL) + { + memory.deallocate(memory.user_reference, payload.size, payload.data); + } } static inline void memZero(const size_t size, void* const data) @@ -1081,13 +1084,18 @@ static inline bool rxSlotEject(size_t* const out_payload_size, { result = true; *out_payload_size = eject_ctx.retain_size; - *out_payload_head = (eject_ctx.head != NULL) - ? (*eject_ctx.head) // Slice off the derived type fields as they are not needed. - : (struct UdpardFragment){.next = NULL, .view = {0, NULL}, .origin = {0, NULL}}; - // This is the single-frame transfer optimization suggested by Scott: we free the first fragment handle - // early by moving the contents into the rx_transfer structure by value. - // No need to free the payload buffer because it has been transferred to the transfer. - memFree(memory.fragment, sizeof(RxFragment), eject_ctx.head); // May be empty. + if (eject_ctx.head != NULL) + { + // This is the single-frame transfer optimization suggested by Scott: we free the first fragment handle + // early by moving the contents into the rx_transfer structure by value. + // No need to free the payload buffer because it has been transferred to the transfer. + *out_payload_head = *eject_ctx.head; // Slice off the derived type fields as they are not needed. + memFree(memory.fragment, sizeof(RxFragment), eject_ctx.head); + } + else + { + *out_payload_head = (struct UdpardFragment){.next = NULL, .view = {0, NULL}, .origin = {0, NULL}}; + } } else // The transfer turned out to be invalid. We have to free the fragments. Can't use the tree anymore. { diff --git a/tests/.clang-tidy b/tests/.clang-tidy index 8a46f65..1e318f0 100644 --- a/tests/.clang-tidy +++ b/tests/.clang-tidy @@ -41,6 +41,9 @@ Checks: >- -modernize-macro-to-enum, -modernize-use-trailing-return-type, -cppcoreguidelines-owning-memory, + -misc-include-cleaner, + -performance-avoid-endl, + -cppcoreguidelines-avoid-do-while, WarningsAsErrors: '*' HeaderFilterRegex: '.*\.hpp' FormatStyle: file