diff --git a/score/mw/com/impl/configuration/config_parser.cpp b/score/mw/com/impl/configuration/config_parser.cpp index a97fa98db..2b2f2fef3 100644 --- a/score/mw/com/impl/configuration/config_parser.cpp +++ b/score/mw/com/impl/configuration/config_parser.cpp @@ -44,6 +44,12 @@ namespace using std::string_view_literals::operator""sv; +// TM: Repeated key definitions in many other files +// binding_service_type_deployment_impl.h +// config_parser.cpp +// lola_service_instance_deployment.cpp +// someip_service_instance_deployment.cpp +// tracing_filter_config_parser.cpp constexpr auto kServiceInstancesKey = "serviceInstances"sv; constexpr auto kInstanceSpecifierKey = "instanceSpecifier"sv; constexpr auto kServiceTypeNameKey = "serviceTypeName"sv; @@ -73,10 +79,10 @@ constexpr auto kEventMaxSubscribersKey = "maxSubscribers"sv; constexpr auto kEventEnforceMaxSamplesKey = "enforceMaxSamples"sv; constexpr auto kEventMaxConcurrentAllocationsKey = "maxConcurrentAllocations"sv; constexpr auto kMaxConcurrentAllocationsDefault = static_cast(1U); -constexpr auto kFieldNumberOfSampleSlotsKey = "numberOfSampleSlots"sv; +constexpr auto kFieldNumberOfSampleSlotsKey = "numberOfSampleSlots"sv; //TM: duplicate of kEventNumberOfSampleSlotsKey, Line 71 constexpr auto kFieldMaxSubscribersKey = "maxSubscribers"sv; constexpr auto kFieldEnforceMaxSamplesKey = "enforceMaxSamples"sv; -constexpr auto kFieldMaxConcurrentAllocationsKey = "maxConcurrentAllocations"sv; +constexpr auto kFieldMaxConcurrentAllocationsKey = "maxConcurrentAllocations"sv; //TM: duplicate of kEventMaxConcurrentAllocationsKey, Line 75 constexpr auto kLolaShmSizeKey = "shm-size"sv; constexpr auto kLolaControlAsilBShmSizeKey = "control-asil-b-shm-size"sv; constexpr auto kLolaControlQmShmSizeKey = "control-qm-shm-size"sv; @@ -456,7 +462,7 @@ auto ParseLolaFieldInstanceDeployment(const score::json::Object& json_map, LolaS const auto number_of_sample_slots = deployment_parser.RetrieveJsonElement( - kFieldNumberOfSampleSlotsKey); + kFieldNumberOfSampleSlotsKey); // different handling than Events which xor-ing with numberOfSampleSlots (using GetNumberOfSampleSlots()) Line 405 const auto max_subscribers = deployment_parser.RetrieveJsonElement( kFieldMaxSubscribersKey); @@ -488,7 +494,6 @@ auto ParseLolaMethodInstanceDeployment(const score::json::Object& json_map, Lola { return; } - const auto methods_list_result = methods->second.As(); SCORE_LANGUAGE_FUTURECPP_PRECONDITION_PRD_MESSAGE(methods_list_result.has_value(), "Configuration corrupted, check with json schema"); const auto& methods_list = methods_list_result.value().get(); @@ -669,7 +674,7 @@ auto ParseServiceInstanceDeployments(const score::json::Object& json_map, "Configuration corrupted, check with json schema"); const auto& bindingValue = bindingValue_result.value().get(); if (bindingValue == kSomeIpBinding) - { + { // TM: Here it logs fatal and aborts, while in 974 ParseServiceTypeDeployment() it just skips it score::mw::log::LogFatal("lola") << "Provided SOME/IP binding, which can not be parsed."; SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(false); } @@ -931,7 +936,7 @@ auto ParseLoLaServiceTypeDeployments(const score::json::Object& json_map) -> Lol score::mw::log::LogFatal("lola") << "Configuration should contain at least one event, field, or method."; SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(false); } - if (!AreEventFieldAndMethodIdsUnique(lola)) + if (!AreEventFieldAndMethodIdsUnique(lola)) { score::mw::log::LogFatal("lola") << "Configuration cannot contain duplicate eventId, fieldId, or methodId."; SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(false); @@ -1064,7 +1069,7 @@ auto ParseSenderQueueSize(const score::json::Object& global_config_map) -> score auto queue_size_obj = queue_size->second.As(); SCORE_LANGUAGE_FUTURECPP_PRECONDITION_PRD_MESSAGE(queue_size_obj.has_value(), "Configuration corrupted, check with json schema"); const auto& queue_size_map = queue_size_obj.value().get(); - const auto& asil_tx_queue_size = queue_size_map.find("B-sender"); + const auto& asil_tx_queue_size = queue_size_map.find("B-sender"); //TM: No QM-sender if (asil_tx_queue_size != queue_size_map.cend()) { return score::ResultToAmpOptionalOrElse(asil_tx_queue_size->second.As(), [](const auto&) { @@ -1296,7 +1301,7 @@ void CrosscheckServiceInstancesToTypes(const Configuration& config) const auto& serviceTypeDeployment = std::get(foundServiceType->second.binding_info_); const auto search = serviceTypeDeployment.events_.find(eventInstanceElement.first); - if (search == serviceTypeDeployment.events_.cend()) + if (search == serviceTypeDeployment.events_.cend()) // TM: there was no check for name duplication during parsing, only ids { ::score::mw::log::LogFatal("lola") << "Service instance " << service_instance.first << "event" << eventInstanceElement.first diff --git a/score/mw/com/impl/configuration/lola_event_instance_deployment.cpp b/score/mw/com/impl/configuration/lola_event_instance_deployment.cpp index 024018bda..e9855f6d8 100644 --- a/score/mw/com/impl/configuration/lola_event_instance_deployment.cpp +++ b/score/mw/com/impl/configuration/lola_event_instance_deployment.cpp @@ -34,6 +34,7 @@ constexpr LolaEventInstanceDeployment::TracingSlotSizeType kNumberOfIpcTracingSl } // namespace +// TM: Constructor is not checking the values (were not also checked in config_parser.cpp, e.g. number_of_sample_slots_) LolaEventInstanceDeployment::LolaEventInstanceDeployment(std::optional number_of_sample_slots, std::optional max_subscribers, std::optional max_concurrent_allocations, @@ -146,6 +147,8 @@ auto LolaEventInstanceDeployment::GetNumberOfTracingSlots() const noexcept -> Tr return number_of_tracing_slots_; } +// TM: SetNumberOfSampleSlots() is not validating the passed value ( 0 < value <= maxSamples). +// Is this intended that it overwrites the json config value? if yes, at least a check against 0 value is needed void LolaEventInstanceDeployment::SetNumberOfSampleSlots(SampleSlotCountType number_of_sample_slots) noexcept { diff --git a/score/mw/com/impl/instance_identifier.cpp b/score/mw/com/impl/instance_identifier.cpp index bcfd83491..de32f7424 100644 --- a/score/mw/com/impl/instance_identifier.cpp +++ b/score/mw/com/impl/instance_identifier.cpp @@ -87,7 +87,7 @@ InstanceIdentifier::InstanceIdentifier(const json::Object& json_object, std::str if (serialization_version != serializationVersion) { ::score::mw::log::LogFatal("lola") << "InstanceIdentifier serialization versions don't match. " - << serialization_version << "!=" << serializationVersion << ". Terminating."; + << serialization_version << "!=" << serializationVersion << ". Terminating."; std::terminate(); } @@ -96,17 +96,18 @@ InstanceIdentifier::InstanceIdentifier(const json::Object& json_object, std::str ServiceTypeDeployment type_deployment{GetValueFromJson(json_object, kServiceTypeDeploymentKey)}; auto service_identifier_type = instance_deployment.service_; + // TM: is this a DeadCode Constructor? What is its use? const auto* const type_deployment_ptr = configuration_->AddServiceTypeDeployment(std::move(service_identifier_type), std::move(type_deployment)); SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(type_deployment_ptr != nullptr, - "Could not insert service type deployment into configuration map"); + "Could not insert service type deployment into configuration map"); type_deployment_ = type_deployment_ptr; auto instance_specifier = instance_deployment.instance_specifier_; const auto* const service_instance_deployment_ptr = configuration_->AddServiceInstanceDeployments(std::move(instance_specifier), std::move(instance_deployment)); SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(service_instance_deployment_ptr != nullptr, - "Could not insert instance deployment into configuration map"); + "Could not insert instance deployment into configuration map"); instance_deployment_ = service_instance_deployment_ptr; } @@ -123,6 +124,7 @@ InstanceIdentifier::InstanceIdentifier(const ServiceInstanceDeployment& deployme { } +// TM: What are the use cases of all Serialize() functions auto InstanceIdentifier::Serialize() const noexcept -> json::Object { json::Object json_object{}; diff --git a/score/mw/com/impl/runtime.cpp b/score/mw/com/impl/runtime.cpp index 09ca8273d..46cdf5d0c 100644 --- a/score/mw/com/impl/runtime.cpp +++ b/score/mw/com/impl/runtime.cpp @@ -35,7 +35,7 @@ namespace inline void warn_double_init() { score::mw::log::LogWarn("lola") << "score::mw::com::impl::Runtime is already initialized! Redundant call to a " - "Runtime::Initialize() overload within production code needs to be checked."; + "Runtime::Initialize() overload within production code needs to be checked."; } inline void error_double_init() @@ -97,7 +97,8 @@ score::cpp::optional ParseTraceConfig(const Configuration& score::Result tracing_config_result = tracing::Parse(trace_filter_config_path, configuration); if (!tracing_config_result.has_value()) { - score::mw::log::LogError("lola") << "Parsing tracing config failed with error: " << tracing_config_result.error(); + score::mw::log::LogError("lola") << "Parsing tracing config failed with error: " + << tracing_config_result.error(); return {}; } return std::move(tracing_config_result).value(); @@ -190,10 +191,11 @@ Runtime::Runtime(std::pairGetTracingRuntime() != nullptr, - "Binding specific runtime has no tracing runtime although tracing is enabled!"); + SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE( + binding_runtime.second->GetTracingRuntime() != nullptr, + "Binding specific runtime has no tracing runtime although tracing is enabled!"); score::cpp::ignore = binding_tracing_runtimes.emplace(binding_runtime.first, - binding_runtime.second->GetTracingRuntime()); + binding_runtime.second->GetTracingRuntime()); } tracing_runtime_ = std::make_unique(std::move(binding_tracing_runtimes)); } @@ -217,6 +219,8 @@ std::vector Runtime::resolve(const InstanceSpecifier& specif // instance is available, there is also a matching service type available. Because parsing of the configuration // is automatically done before instantiating the runtime, this condition is always positive. To increase the // robustness of the code, we still check for this condition. + + // TM: Will we remove such extra robustness check? if (type_deployment != configuration_.GetServiceTypes().cend()) { result.push_back(make_InstanceIdentifier(instanceSearch->second, type_deployment->second)); diff --git a/score/mw/com/impl/service_discovery.cpp b/score/mw/com/impl/service_discovery.cpp index 2d28efaf6..bb4e6bd9d 100644 --- a/score/mw/com/impl/service_discovery.cpp +++ b/score/mw/com/impl/service_discovery.cpp @@ -300,7 +300,8 @@ auto ServiceDiscovery::GetServiceDiscoveryClient(const InstanceIdentifier& insta mw::log::LogFatal("lola") << "Service discovery failed to find fitting binding for" << instance_identifier.ToString(); } - SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(binding_runtime != nullptr, "Unsupported binding"); + // TM: This assertion is redundant (Line 298) + SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(binding_runtime != nullptr, "Unsupported binding"); return binding_runtime->GetServiceDiscoveryClient(); }