Skip to content

Commit

Permalink
Don't call memory.deallocate for NULL memory (#59)
Browse files Browse the repository at this point in the history
Also:
- Latest `toolshed:ts22.4.3` -> `22.4.10` : gcc 11.3 -> 12.3
- Disable some new clang-tidies
  • Loading branch information
serges147 authored Aug 16, 2024
1 parent 6c7c12e commit 3d9f5ef
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 12 deletions.
4 changes: 4 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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' ]
Expand Down Expand Up @@ -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' ]
Expand All @@ -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++
Expand Down Expand Up @@ -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')) &&
Expand Down
24 changes: 16 additions & 8 deletions libudpard/udpard.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
{
Expand Down
3 changes: 3 additions & 0 deletions tests/.clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 3d9f5ef

Please sign in to comment.