Skip to content

Commit bccd1b3

Browse files
committed
lola: Pass reference to EventDataControlComposite
Since we always create a QM EventDataControl, we pass it into the composite by reference instead of by pointer to avoid doing nullptr checks within the composite (and for clearer semantics).
1 parent 6b69fb6 commit bccd1b3

File tree

8 files changed

+42
-59
lines changed

8 files changed

+42
-59
lines changed

score/mw/com/impl/bindings/lola/control_slot_indicator.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
namespace score::mw::com::impl::lola
2323
{
2424

25+
/// GTODO: Remove!!!
2526
/// \brief Helper class which identifies a slot in our "control slot array" using both the slot index and a raw pointer
2627
/// to the element.
2728
/// \details We combine identification by the index of the slot and a (raw) pointer to the slot. Reasoning:

score/mw/com/impl/bindings/lola/event_data_control_composite.cpp

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -27,23 +27,22 @@ constexpr std::size_t MAX_MULTI_ALLOCATE_RETRY_COUNT{100U};
2727
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-member-init): all members are initialized in the delegated constructor
2828
template <template <class> class AtomicIndirectorType>
2929
EventDataControlComposite<AtomicIndirectorType>::EventDataControlComposite(
30-
SkeletonEventDataControlLocalView<>* const asil_qm_control_local,
30+
SkeletonEventDataControlLocalView<>& asil_qm_control_local,
3131
ProxyEventDataControlLocalView<>* const proxy_control_local)
3232
: EventDataControlComposite{asil_qm_control_local, nullptr, proxy_control_local}
3333
{
3434
}
3535

3636
template <template <class> class AtomicIndirectorType>
3737
EventDataControlComposite<AtomicIndirectorType>::EventDataControlComposite(
38-
SkeletonEventDataControlLocalView<>* const asil_qm_control_local,
38+
SkeletonEventDataControlLocalView<>& asil_qm_control_local,
3939
SkeletonEventDataControlLocalView<>* const asil_b_control_local,
4040
ProxyEventDataControlLocalView<>* const proxy_control_local)
4141
: asil_qm_control_local_{asil_qm_control_local},
4242
asil_b_control_local_{asil_b_control_local},
4343
proxy_control_local_{proxy_control_local},
4444
ignore_qm_control_{false}
4545
{
46-
CheckForValidDataControls();
4746
}
4847

4948
template <template <class> class AtomicIndirectorType>
@@ -82,7 +81,7 @@ auto EventDataControlComposite<AtomicIndirectorType>::GetNextFreeMultiSlot() con
8281
for (auto [current_index, it_slots_qm, it_slots_asil_b] = std::make_tuple(
8382
// coverity[autosar_cpp14_a5_2_2_violation]
8483
std::size_t{0U},
85-
asil_qm_control_local_->state_slots_.begin(),
84+
asil_qm_control_local_.get().state_slots_.begin(),
8685
asil_b_control_local_->state_slots_.begin());
8786
current_index != asil_b_control_local_->state_slots_.size();
8887
// coverity[autosar_cpp14_m6_5_5_violation]
@@ -116,10 +115,7 @@ auto EventDataControlComposite<AtomicIndirectorType>::GetNextFreeMultiSlot() con
116115
{
117116
return {possible_index.value(), *qm_slot_ptr, *asil_b_slot_ptr};
118117
}
119-
else
120-
{
121-
return {};
122-
}
118+
return {};
123119
}
124120

125121
template <template <class> class AtomicIndirectorType>
@@ -196,7 +192,7 @@ auto EventDataControlComposite<AtomicIndirectorType>::AllocateNextSlot() noexcep
196192
{
197193
if (asil_b_control_local_ == nullptr)
198194
{
199-
auto qm_control_slot_indicator = asil_qm_control_local_->AllocateNextSlot();
195+
auto qm_control_slot_indicator = asil_qm_control_local_.get().AllocateNextSlot();
200196
if (qm_control_slot_indicator.IsValid())
201197
{
202198
return {qm_control_slot_indicator.GetIndex(),
@@ -251,7 +247,7 @@ auto EventDataControlComposite<AtomicIndirectorType>::EventReady(ControlSlotComp
251247

252248
if (!ignore_qm_control_)
253249
{
254-
asil_qm_control_local_->EventReady({slot_indicator.GetIndex(), slot_indicator.GetSlotQM()}, time_stamp);
250+
asil_qm_control_local_.get().EventReady({slot_indicator.GetIndex(), slot_indicator.GetSlotQM()}, time_stamp);
255251
}
256252
}
257253

@@ -265,7 +261,7 @@ auto EventDataControlComposite<AtomicIndirectorType>::Discard(ControlSlotComposi
265261

266262
if (!ignore_qm_control_)
267263
{
268-
asil_qm_control_local_->Discard({slot_indicator.GetIndex(), slot_indicator.GetSlotQM()});
264+
asil_qm_control_local_.get().Discard({slot_indicator.GetIndex(), slot_indicator.GetSlotQM()});
269265
}
270266
}
271267

@@ -279,7 +275,7 @@ template <template <class> class AtomicIndirectorType>
279275
SkeletonEventDataControlLocalView<>& EventDataControlComposite<AtomicIndirectorType>::GetQmEventDataControlLocal()
280276
const noexcept
281277
{
282-
return *asil_qm_control_local_;
278+
return asil_qm_control_local_;
283279
}
284280

285281
template <template <class> class AtomicIndirectorType>
@@ -290,7 +286,8 @@ EventDataControlComposite<AtomicIndirectorType>::GetAsilBEventDataControlLocal()
290286
}
291287

292288
template <template <class> class AtomicIndirectorType>
293-
ProxyEventDataControlLocalView<>& EventDataControlComposite<AtomicIndirectorType>::GetProxyEventDataControlLocalView() noexcept
289+
ProxyEventDataControlLocalView<>&
290+
EventDataControlComposite<AtomicIndirectorType>::GetProxyEventDataControlLocalView() noexcept
294291
{
295292
SCORE_LANGUAGE_FUTURECPP_PRECONDITION_PRD(proxy_control_local_ != nullptr);
296293
return *proxy_control_local_;
@@ -308,21 +305,12 @@ EventSlotStatus::EventTimeStamp EventDataControlComposite<AtomicIndirectorType>:
308305
}
309306
else
310307
{
311-
const EventSlotStatus event_slot_status{(*asil_qm_control_local_)[slot]};
308+
const EventSlotStatus event_slot_status{asil_qm_control_local_.get()[slot]};
312309
const EventSlotStatus::EventTimeStamp sample_timestamp{event_slot_status.GetTimeStamp()};
313310
return sample_timestamp;
314311
}
315312
}
316313

317-
template <template <class> class AtomicIndirectorType>
318-
void EventDataControlComposite<AtomicIndirectorType>::CheckForValidDataControls() const noexcept
319-
{
320-
if (asil_qm_control_local_ == nullptr)
321-
{
322-
std::terminate();
323-
}
324-
}
325-
326314
template <template <class> class AtomicIndirectorType>
327315
// Suppress "AUTOSAR C++14 A15-5-3" rule findings. This rule states: "The std::terminate() function shall not be called
328316
// implicitly". std::terminate() is implicitly called from 'state_slots_[]' which might leds to a segmentation fault
@@ -332,17 +320,17 @@ template <template <class> class AtomicIndirectorType>
332320
EventSlotStatus::EventTimeStamp EventDataControlComposite<AtomicIndirectorType>::GetLatestTimestamp() const noexcept
333321
{
334322
EventSlotStatus::EventTimeStamp latest_time_stamp{1U};
335-
SkeletonEventDataControlLocalView<>* control =
336-
(asil_b_control_local_ != nullptr) ? asil_b_control_local_ : asil_qm_control_local_;
323+
SkeletonEventDataControlLocalView<>& control =
324+
(asil_b_control_local_ != nullptr) ? *asil_b_control_local_ : asil_qm_control_local_.get();
337325
for (SlotIndexType slot_index = 0U;
338326
// Suppress "AUTOSAR C++14 A4-7-1" rule finding. This rule states: "An integer expression shall not lead to
339327
// loss.". As the maximum number of slots is std::uint16_t, so there is no case for a data loss here.
340328
// coverity[autosar_cpp14_a4_7_1_violation]
341-
slot_index < static_cast<SlotIndexType>(control->state_slots_.size());
329+
slot_index < static_cast<SlotIndexType>(control.state_slots_.size());
342330
++slot_index)
343331
{
344-
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(static_cast<std::size_t>(slot_index) < control->state_slots_.size());
345-
const EventSlotStatus slot{control->state_slots_[slot_index].load(std::memory_order_acquire)};
332+
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(static_cast<std::size_t>(slot_index) < control.state_slots_.size());
333+
const EventSlotStatus slot{control.state_slots_[slot_index].load(std::memory_order_acquire)};
346334
if (!slot.IsInvalid() && !slot.IsInWriting())
347335
{
348336
const auto slot_time_stamp = slot.GetTimeStamp();

score/mw/com/impl/bindings/lola/event_data_control_composite.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,13 @@
1515

1616
#include "score/mw/com/impl/bindings/lola/control_slot_composite_indicator.h"
1717
#include "score/mw/com/impl/bindings/lola/control_slot_types.h"
18-
#include "score/mw/com/impl/bindings/lola/event_data_control.h"
1918
#include "score/mw/com/impl/bindings/lola/event_slot_status.h"
2019
#include "score/mw/com/impl/bindings/lola/proxy_event_data_control_local_view.h"
2120
#include "score/mw/com/impl/bindings/lola/skeleton_event_data_control_local_view.h"
2221

2322
#include "score/memory/shared/atomic_indirector.h"
2423

25-
#include <optional>
24+
#include <functional>
2625

2726
namespace score::mw::com::impl::lola
2827
{
@@ -47,11 +46,11 @@ class EventDataControlComposite
4746

4847
public:
4948
/// \brief Constructs a composite which will only manage a single QM control (no ASIL use-case)
50-
explicit EventDataControlComposite(SkeletonEventDataControlLocalView<>* const asil_qm_control_local,
49+
explicit EventDataControlComposite(SkeletonEventDataControlLocalView<>& asil_qm_control_local,
5150
ProxyEventDataControlLocalView<>* const proxy_control_local);
5251

5352
/// \brief Constructs a composite which will manage QM and ASIL control at the same time
54-
explicit EventDataControlComposite(SkeletonEventDataControlLocalView<>* const asil_qm_control_local,
53+
explicit EventDataControlComposite(SkeletonEventDataControlLocalView<>& asil_qm_control_local,
5554
SkeletonEventDataControlLocalView<>* const asil_b_control_local,
5655
ProxyEventDataControlLocalView<>* const proxy_control_local);
5756

@@ -104,7 +103,7 @@ class EventDataControlComposite
104103
EventSlotStatus::EventTimeStamp GetLatestTimestamp() const noexcept;
105104

106105
private:
107-
SkeletonEventDataControlLocalView<>* asil_qm_control_local_;
106+
std::reference_wrapper<SkeletonEventDataControlLocalView<>> asil_qm_control_local_;
108107
SkeletonEventDataControlLocalView<>* asil_b_control_local_;
109108

110109
ProxyEventDataControlLocalView<>* proxy_control_local_;

score/mw/com/impl/bindings/lola/event_data_control_composite_test.cpp

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ class EventDataControlCompositeFixture : public ::testing::Test
107107
SCORE_LANGUAGE_FUTURECPP_ASSERT(skeleton_qm_local_.has_value());
108108

109109
auto* const asil_control = skeleton_asil_local_.has_value() ? &skeleton_asil_local_.value() : nullptr;
110-
unit_ = std::make_unique<EventDataControlComposite<>>(&skeleton_qm_local_.value(), asil_control, nullptr);
110+
unit_ = std::make_unique<EventDataControlComposite<>>(skeleton_qm_local_.value(), asil_control, nullptr);
111111
return *this;
112112
}
113113

@@ -120,7 +120,7 @@ class EventDataControlCompositeFixture : public ::testing::Test
120120

121121
auto* const asil_control = skeleton_asil_local_.has_value() ? &skeleton_asil_local_.value() : nullptr;
122122
unit_mock_ = std::make_unique<EventDataControlComposite<memory::shared::AtomicIndirectorMock>>(
123-
&skeleton_qm_local_.value(), asil_control, nullptr);
123+
skeleton_qm_local_.value(), asil_control, nullptr);
124124

125125
return *this;
126126
}
@@ -597,7 +597,7 @@ TEST(EventDataControlCompositeTest, DISABLED_fuzz)
597597
SkeletonEventDataControlLocalView skeleton_asil_local{asil};
598598
SkeletonEventDataControlLocalView skeleton_qm_local{qm};
599599

600-
EventDataControlComposite unit{&skeleton_qm_local, &skeleton_asil_local, nullptr};
600+
EventDataControlComposite unit{skeleton_qm_local, &skeleton_asil_local, nullptr};
601601

602602
std::mutex allocated_slots_mutex{};
603603
std::vector<ControlSlotCompositeIndicator> allocated_slots{};
@@ -765,7 +765,7 @@ TEST_F(EventDataControlCompositeFixture, GetProxyEventDataControlLocalView)
765765
EventDataControl qm{kMaxSlots, memory_, kMaxSubscribers};
766766
SkeletonEventDataControlLocalView<> skeleton_qm_local{qm};
767767
ProxyEventDataControlLocalView<> proxy_qm_local{qm};
768-
EventDataControlComposite<> unit{&skeleton_qm_local, &proxy_qm_local};
768+
EventDataControlComposite<> unit{skeleton_qm_local, &proxy_qm_local};
769769

770770
// When getting the ASIL-B event data control
771771
auto& returned_proxy_qm_local = unit.GetProxyEventDataControlLocalView();
@@ -782,7 +782,7 @@ TEST_F(EventDataControlCompositeFixture, GetProxyEventDataControlLocalViewWithAs
782782
SkeletonEventDataControlLocalView<> skeleton_qm_local{qm};
783783
SkeletonEventDataControlLocalView<> skeleton_asil_b_local{asil_b};
784784
ProxyEventDataControlLocalView<> proxy_qm_local{qm};
785-
EventDataControlComposite<> unit{&skeleton_qm_local, &skeleton_asil_b_local, &proxy_qm_local};
785+
EventDataControlComposite<> unit{skeleton_qm_local, &skeleton_asil_b_local, &proxy_qm_local};
786786

787787
// When getting the ASIL-B event data control
788788
auto& returned_proxy_qm_local = unit.GetProxyEventDataControlLocalView();
@@ -797,18 +797,13 @@ TEST_F(EventDataControlCompositeFixture,
797797
// Given an EventDataControlComposite constructed without a ProxyEventDataControlLocalView
798798
EventDataControl qm{kMaxSlots, memory_, kMaxSubscribers};
799799
SkeletonEventDataControlLocalView<> skeleton_qm_local{qm};
800-
EventDataControlComposite<> unit{&skeleton_qm_local, nullptr};
800+
EventDataControlComposite<> unit{skeleton_qm_local, nullptr};
801801

802802
// When getting the ASIL-B event data control
803803
// Then the program terminates
804804
EXPECT_DEATH(score::cpp::ignore = unit.GetProxyEventDataControlLocalView(), ".*");
805805
}
806806

807-
TEST(EventDataControlCompositeDeathTest, DiesOnConstructionWithNullptr)
808-
{
809-
EXPECT_DEATH(EventDataControlComposite(nullptr, nullptr), ".*");
810-
}
811-
812807
using EventDataControlCompositeGetTimestampFixture = EventDataControlCompositeFixture;
813808
TEST_F(EventDataControlCompositeGetTimestampFixture, CanAllocateOneSlot)
814809
{

score/mw/com/impl/bindings/lola/event_data_control_test_resources.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ void EventDataControlCompositeAttorney::PrepareAllocateNextSlot(
3232
// we want, that event_data_control_composite_ returns slot_index from call to AllocateNextSlot().
3333
// To achieve this, we set the reference-count of all slots but slot_index to a value > 0.
3434
EventSlotStatus::EventTimeStamp timestamp{1};
35-
const auto number_states_slots = event_data_control_composite_.asil_qm_control_local_->state_slots_.size();
35+
const auto number_states_slots = event_data_control_composite_.asil_qm_control_local_.get().state_slots_.size();
3636
for (std::size_t index{0}; index < number_states_slots; index++)
3737
{
3838
EventSlotStatus status{};
@@ -46,7 +46,7 @@ void EventDataControlCompositeAttorney::PrepareAllocateNextSlot(
4646
{
4747
status.SetReferenceCount(1U);
4848
}
49-
event_data_control_composite_.asil_qm_control_local_->state_slots_[index].store(
49+
event_data_control_composite_.asil_qm_control_local_.get().state_slots_[index].store(
5050
static_cast<EventSlotStatus::value_type>(status));
5151
if (event_data_control_composite_.asil_b_control_local_ != nullptr)
5252
{
@@ -64,7 +64,7 @@ void EventDataControlCompositeAttorney::SetQmControlDisconnected(const bool expe
6464
std::pair<EventSlotStatus, score::cpp::optional<EventSlotStatus>> EventDataControlCompositeAttorney::GetSlotStatus(
6565
const SlotIndexType slot_index) const noexcept
6666
{
67-
return {EventSlotStatus(event_data_control_composite_.asil_qm_control_local_->state_slots_[slot_index]),
67+
return {EventSlotStatus(event_data_control_composite_.asil_qm_control_local_.get().state_slots_[slot_index]),
6868
event_data_control_composite_.asil_b_control_local_ != nullptr
6969
? score::cpp::optional<EventSlotStatus>(
7070
event_data_control_composite_.asil_b_control_local_->state_slots_[slot_index])

score/mw/com/impl/bindings/lola/sample_allocatee_ptr_test.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class SampleAllocateePtrFixture : public ::testing::Test
4141
FakeMemoryResource memory_{};
4242
EventDataControl control_block_{kMaxSlots, memory_, kMaxSubscribers};
4343
SkeletonEventDataControlLocalView<> skeleton_event_data_control_local_{control_block_};
44-
EventDataControlComposite<> control_composite_{&skeleton_event_data_control_local_, nullptr};
44+
EventDataControlComposite<> control_composite_{skeleton_event_data_control_local_, nullptr, nullptr};
4545
};
4646

4747
TEST_F(SampleAllocateePtrFixture, PtrContainingInvalidSlotIsNotDestroyingAnything)

score/mw/com/impl/bindings/lola/skeleton.h

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -418,14 +418,14 @@ auto Skeleton::OpenEventDataFromOpenedSharedMemory(const ElementFqId element_fq_
418418
// The "event_data_control_asil_b" and "typed_event_data_storage_ptr" are still valid lifetime even returned pointer
419419
// to internal state until Skeleton object is alive.
420420
// coverity[autosar_cpp14_a3_8_1_violation]
421-
return {typed_event_data_storage_ptr,
422-
// The lifetime of the "event_data_control_asil_b" object lasts as long as the Skeleton is alive.
423-
// coverity[autosar_cpp14_m7_5_1_violation]
424-
// coverity[autosar_cpp14_m7_5_2_violation]
425-
// coverity[autosar_cpp14_a3_8_1_violation]
426-
EventDataControlComposite{&event_control_qm_it->second.data_control,
427-
event_data_control_asil_b_local,
428-
proxy_event_data_control_local}};
421+
return {
422+
typed_event_data_storage_ptr,
423+
// The lifetime of the "event_data_control_asil_b" object lasts as long as the Skeleton is alive.
424+
// coverity[autosar_cpp14_m7_5_1_violation]
425+
// coverity[autosar_cpp14_m7_5_2_violation]
426+
// coverity[autosar_cpp14_a3_8_1_violation]
427+
EventDataControlComposite{
428+
event_control_qm_it->second.data_control, event_data_control_asil_b_local, proxy_event_data_control_local}};
429429
}
430430

431431
template <typename SampleType>
@@ -500,7 +500,7 @@ auto Skeleton::CreateEventDataFromOpenedSharedMemory(const ElementFqId element_f
500500
// coverity[autosar_cpp14_m7_5_2_violation]
501501
// coverity[autosar_cpp14_a3_8_1_violation]
502502
return {typed_event_data_storage_ptr,
503-
EventDataControlComposite{&skeleton_event_data_control_local_qm.get().data_control,
503+
EventDataControlComposite{skeleton_event_data_control_local_qm.get().data_control,
504504
control_asil_local_result,
505505
proxy_event_data_control_local}};
506506
}

score/mw/com/impl/plumbing/sample_allocatee_ptr_test.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ class SampleAllocateePtrFixture : public ::testing::Test
5252
lola::FakeMemoryResource fake_memory_resource_{};
5353
lola::EventDataControl event_data_ctrl_qm_{0, fake_memory_resource_, 1U};
5454
lola::SkeletonEventDataControlLocalView<> skeleton_event_data_ctrl_qm_local_{event_data_ctrl_qm_};
55-
lola::EventDataControlComposite<> event_data_ctrl_{&skeleton_event_data_ctrl_qm_local_, nullptr};
55+
lola::EventDataControlComposite<> event_data_ctrl_{skeleton_event_data_ctrl_qm_local_, nullptr};
5656
lola::SlotIndexType event_data_slot_index_{std::numeric_limits<lola::SlotIndexType>::max()};
5757
lola::SampleAllocateePtr<std::uint8_t> lola_allocatee_ptr_{&value_, event_data_ctrl_, {}};
5858
SampleAllocateePtr<std::uint8_t> valid_unit_{MakeSampleAllocateePtr(std::move(lola_allocatee_ptr_))};
@@ -504,7 +504,7 @@ TEST_F(SampleAllocateePtrFixture, UnderlyingLolaPtrIsFreedOnDestruction)
504504
RecordProperty("DerivationTechnique", "Analysis of requirements");
505505

506506
// Given a SampleAllocateePtr with an underlying lola SampleAllocateePtr
507-
lola::EventDataControlComposite event_data_ctrl{&skeleton_event_data_ctrl_qm_local_, nullptr};
507+
lola::EventDataControlComposite event_data_ctrl{skeleton_event_data_ctrl_qm_local_, nullptr};
508508

509509
bool is_destructed{false};
510510
ObjectDestructionNotifier object_destruction_notifier(is_destructed);

0 commit comments

Comments
 (0)