Skip to content

Commit a788dea

Browse files
serges147serges147
committed
Fix unit tests
1 parent 2443c94 commit a788dea

File tree

5 files changed

+77
-40
lines changed

5 files changed

+77
-40
lines changed

‎include/libcyphal/transport/can/can_transport_impl.hpp‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,8 @@ class TransportImpl final : private TransportDelegate, public ICanTransport
147147
return ArgumentError{};
148148
}
149149

150-
// False positive of clang-tidy - we move `media_array` to the `transport` instance, so can't make it const.
151-
// NOLINTNEXTLINE(misc-const-correctness)
152-
MediaArray media_array = makeMediaArray(memory, media_count, media, tx_capacity);
153-
if (media_array.size() != media_count)
150+
MediaArray media_array{media_count, &memory};
151+
if (!makeMediaArray(media_array, media_count, media, tx_capacity))
154152
{
155153
return MemoryError{};
156154
}
@@ -514,18 +512,23 @@ class TransportImpl final : private TransportDelegate, public ICanTransport
514512
return tryHandleTransientFailure<Report>(std::move(*failure), media.index(), canardInstance());
515513
}
516514

517-
CETL_NODISCARD static MediaArray makeMediaArray(cetl::pmr::memory_resource& memory,
518-
const std::size_t media_count,
519-
const cetl::span<IMedia*> media_interfaces,
520-
const std::size_t tx_capacity)
515+
CETL_NODISCARD static bool makeMediaArray(MediaArray& media_array,
516+
const std::size_t media_count,
517+
const cetl::span<IMedia*> media_interfaces,
518+
const std::size_t tx_capacity)
521519
{
522-
MediaArray media_array{media_count, &memory};
523-
524-
// Reserve the space for the whole array (to avoid reallocations).
525-
// Capacity will be less than requested in case of out of memory.
526-
media_array.reserve(media_count);
527-
if (media_array.capacity() >= media_count)
520+
#if defined(__cpp_exceptions)
521+
try
528522
{
523+
#endif
524+
// Reserve the space for the whole array (to avoid reallocations).
525+
// Capacity will be less than requested in case of out of memory.
526+
media_array.reserve(media_count);
527+
if (media_array.capacity() < media_count)
528+
{
529+
return false;
530+
}
531+
529532
std::size_t index = 0;
530533
for (IMedia* const media_interface : media_interfaces)
531534
{
@@ -538,9 +541,14 @@ class TransportImpl final : private TransportDelegate, public ICanTransport
538541
}
539542
CETL_DEBUG_ASSERT(index == media_count, "");
540543
CETL_DEBUG_ASSERT(media_array.size() == media_count, "");
541-
}
544+
return true;
542545

543-
return media_array;
546+
#if defined(__cpp_exceptions)
547+
} catch (const std::bad_alloc&)
548+
{
549+
return false;
550+
}
551+
#endif
544552
}
545553

546554
static void flushCanardTxQueue(CanardTxQueue& canard_tx_queue, const CanardInstance& canard_instance)

‎include/libcyphal/transport/can/delegate.hpp‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -479,13 +479,23 @@ class TransportDelegate
479479
// Now we know that we have at least one active port,
480480
// so we need preallocate temp memory for the total number of active ports.
481481
//
482-
filters.reserve(total_active_ports);
483-
if (filters.capacity() < total_active_ports)
482+
#if defined(__cpp_exceptions)
483+
try
484+
{
485+
#endif
486+
filters.reserve(total_active_ports);
487+
if (filters.capacity() < total_active_ports)
488+
{
489+
// This is out of memory situation.
490+
return false;
491+
}
492+
493+
#if defined(__cpp_exceptions)
494+
} catch (const std::bad_alloc&)
484495
{
485-
// This is out of memory situation.
486496
return false;
487497
}
488-
498+
#endif
489499
// `ports_count` counting is just for the sake of debug verification.
490500
std::size_t ports_count = 0;
491501

‎include/libcyphal/transport/udp/udp_transport_impl.hpp‎

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,8 @@ class TransportImpl final : private TransportDelegate, public IUdpTransport
164164

165165
const UdpardNodeID unset_node_id = UDPARD_NODE_ID_UNSET;
166166

167-
// False positive of clang-tidy - we move `media_array` to the `transport` instance, so can't make it const.
168-
// NOLINTNEXTLINE(misc-const-correctness)
169-
MediaArray media_array = makeMediaArray(memory_resources, media_count, media, &unset_node_id, tx_capacity);
170-
if (media_array.size() != media_count)
167+
MediaArray media_array{media_count, &memory_resources.general};
168+
if (!makeMediaArray(memory_resources, media_array, media_count, media, &unset_node_id, tx_capacity))
171169
{
172170
return MemoryError{};
173171
}
@@ -618,19 +616,25 @@ class TransportImpl final : private TransportDelegate, public IUdpTransport
618616
return failure;
619617
}
620618

621-
CETL_NODISCARD static MediaArray makeMediaArray(const MemoryResources& memory,
622-
const std::size_t media_count,
623-
const cetl::span<IMedia*> media_interfaces,
624-
const UdpardNodeID* const local_node_id_,
625-
const std::size_t tx_capacity)
619+
CETL_NODISCARD static bool makeMediaArray(const MemoryResources& memory,
620+
MediaArray& media_array,
621+
const std::size_t media_count,
622+
const cetl::span<IMedia*> media_interfaces,
623+
const UdpardNodeID* const local_node_id_,
624+
const std::size_t tx_capacity)
626625
{
627-
MediaArray media_array{media_count, &memory.general};
628-
629-
// Reserve the space for the whole array (to avoid reallocations).
630-
// Capacity will be less than requested in case of out of memory.
631-
media_array.reserve(media_count);
632-
if (media_array.capacity() >= media_count)
626+
#if defined(__cpp_exceptions)
627+
try
633628
{
629+
#endif
630+
// Reserve the space for the whole array (to avoid reallocations).
631+
// Capacity will be less than requested in case of out of memory (or `bad_alloc` if exceptions are enabled).
632+
media_array.reserve(media_count);
633+
if (media_array.capacity() < media_count)
634+
{
635+
return false;
636+
}
637+
634638
std::size_t index = 0;
635639
for (IMedia* const media_interface : media_interfaces)
636640
{
@@ -643,9 +647,14 @@ class TransportImpl final : private TransportDelegate, public IUdpTransport
643647
}
644648
CETL_DEBUG_ASSERT(index == media_count, "");
645649
CETL_DEBUG_ASSERT(media_array.size() == media_count, "");
646-
}
650+
return true;
647651

648-
return media_array;
652+
#if defined(__cpp_exceptions)
653+
} catch (const std::bad_alloc&)
654+
{
655+
return false;
656+
}
657+
#endif
649658
}
650659

651660
/// @brief Tries to run an action with media and its TX socket (the latter one is made on demand if necessary).

‎test/unittest/transport/can/test_can_transport.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,7 +1066,7 @@ TEST_F(TestCanTransport, receive_svc_responses_with_fallible_oom_canard)
10661066
EXPECT_CALL(mr_mock, do_reallocate(nullptr, 0, _, _)).WillRepeatedly(Return(nullptr));
10671067
#endif
10681068

1069-
// 1st run: canard RX has failed to accept frame and there is no transient error handler.
1069+
// 1st run: canard RX has failed to accept frame, and there is no transient error handler.
10701070
scheduler_.scheduleAt(1s, [&](const auto&) {
10711071
//
10721072
scheduler_.scheduleNamedCallback("rx");
@@ -1075,7 +1075,7 @@ TEST_F(TestCanTransport, receive_svc_responses_with_fallible_oom_canard)
10751075
EXPECT_THAT(session->receive(), Eq(cetl::nullopt));
10761076
});
10771077
});
1078-
// 2nd run: canard RX has failed to accept frame and there is "failing" transient error handler.
1078+
// 2nd run: canard RX has failed to accept frame, and there is "failing" transient error handler.
10791079
scheduler_.scheduleAt(2s, [&](const auto&) {
10801080
//
10811081
transport->setTransientErrorHandler(std::ref(handler_mock));
@@ -1086,7 +1086,7 @@ TEST_F(TestCanTransport, receive_svc_responses_with_fallible_oom_canard)
10861086
EXPECT_THAT(session->receive(), Eq(cetl::nullopt));
10871087
});
10881088
});
1089-
// 3rd run: canard RX has failed to accept frame and there is "success" transient error handler -
1089+
// 3rd run: canard RX has failed to accept frame, and there is "success" transient error handler -
10901090
// the received frame should be just dropped, but overall `run` result should be success (aka ignore OOM).
10911091
scheduler_.scheduleAt(3s, [&](const auto&) {
10921092
//

‎test/unittest/transport/scattered_buffer_storage_mock.hpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,22 @@ class ScatteredBufferStorageMock : public ScatteredBuffer::IStorage
6464
{
6565
return (mock_ != nullptr) ? mock_->size() : 0;
6666
}
67+
6768
std::size_t copy(const std::size_t offset_bytes,
6869
cetl::byte* const destination,
6970
const std::size_t length_bytes) const override
7071
{
7172
return (mock_ != nullptr) ? mock_->copy(offset_bytes, destination, length_bytes) : 0;
7273
}
7374

75+
void observeFragments(ScatteredBuffer::IFragmentsObserver& observer) const override
76+
{
77+
if (nullptr != mock_)
78+
{
79+
mock_->observeFragments(observer);
80+
}
81+
}
82+
7483
private:
7584
ScatteredBufferStorageMock* mock_{nullptr};
7685

@@ -100,6 +109,7 @@ class ScatteredBufferStorageMock : public ScatteredBuffer::IStorage
100109

101110
MOCK_METHOD(std::size_t, size, (), (const, noexcept, override)); // NOLINT(bugprone-exception-escape)
102111
MOCK_METHOD(std::size_t, copy, (const std::size_t, cetl::byte* const, const std::size_t), (const, override));
112+
MOCK_METHOD(void, observeFragments, (ScatteredBuffer::IFragmentsObserver & observer), (const, override));
103113

104114
}; // ScatteredBufferStorageMock
105115

0 commit comments

Comments
 (0)