Repository navigation
AoU cleanup - #1099
AoU cleanup#1099
Conversation
b659af5 to
f38fff7
Compare
There was a problem hiding this comment.
This looks wrong. At least some of those AoUs are still relevant to my knowledge.
@crimson11 @bemerybmw please say so if this knowledge is outdated.
I think the correct action is to extend the FTAs and FMEAs respectively to be more thorough.
| mitigates = "TooFewMemoryAllocated, TooMuchMemoryAllocated, SendingEventSendsToWrongConsumer" | ||
| } | ||
|
|
||
| /* broken_link_c/issue/5835324 */ |
There was a problem hiding this comment.
What was your intention to delete this AoU? I guess - as of now it is needed/referenced by an FMEA ("Too few memory allocated"). If we would delete this AoU (see details below), then we would also need to delete the AoU above "OneProducerOnlyOneAllocateePtr", because they are both strongly related.
TBH: I'm also not really happy with this AoU. For several reasons:
- Why shouldn't we allow this? The user is responsible for configuring the slot-count to his needs! If he configures it sufficient, there is no reason, why he can't hold more than one
SampleAllocateePtrin parallel - We even at some point had a config-value (removed now) to explicitly allow the user to configure the number of
SampleAllocateePtr, he wants to hold in parallel (to support him with some supervision). - We can discuss, whether this "supervision" in (2) makes sense or not ... but having this AoU, while we in code NOT checking this "restriction" (if we really want it) in code ALTHOUGH we easily could, makes no sense to me!
Summary
There are imho two options:
A. We keep this AoU and the AoU above "OneProducerOnlyOneAllocateePtr" and also the FTA referencing this AoU as an "preventive measure". This is the "lazy" approach - keeps everything as it is now and it is "consistent".
B. We clean this up: We remove both AoUs and adapt the FMEA/FTA: Getting an error-result in SkeletonEvent::Allocate() or Send(SampleType&) is NOT an error in the case the user himself is "exhausting" the slots on provider side. Then I would rather re-phrase the existing "CorrectlyConfiguredMaximumNumberOfMaximumElementsPerSubscriber" AoU into "CorrectlyConfigurednumberOfSampleSlots", which simply states, that "numberOfSampleSlots" needs to be dimensioned correctly according to consumers AND producer need!
There was a problem hiding this comment.
My intention was to remove this as a superfluous AoU during the cleanup, but I missed that it is referenced by the FMEA and is closely related to OneProducerOnlyOneAllocateePtr.
I agree that removing it in isolation would leave the safety analysis inconsistent. I’ll restore it for this PR. The broader cleanup option—removing both AoUs and updating the FMEA/FTA and slot-count assumption—should be handled as a coordinated follow-up.
There was a problem hiding this comment.
Should be adapted imho! The Allocate() call nowadays returns a Result -> should be checked for an error instead!
| } | ||
|
|
||
| /* broken_link_c/issue/7091801 */ | ||
| ScoreReq.AoU UsageOfConfigurationOversubscription { |
There was a problem hiding this comment.
Why you deleted this? What was your intention?
| } | ||
|
|
||
| /* broken_link_c/issue/21209491 */ | ||
| ScoreReq.AoU EventOrFieldReceptionViaGenericProxyNeedsSpecificCare { |
There was a problem hiding this comment.
Why you removed this? I guess it is still valid ... only the API GetSampleSize is deprecated and is now superseeded by GetDataTypeSizeInfo
There was a problem hiding this comment.
You’re right, the requirement is still valid. I’ve restored it and added a note that {{GetSampleSize()}} is deprecated in favor of {{GetDataTypeSizeInfo()}}, while keeping the original safety requirement intact.
| } | ||
|
|
||
| /* broken_link_c/issue/79416086 */ | ||
| ScoreReq.AoU NoGuaranteeOnSubscriptionStateCorrectness { |
There was a problem hiding this comment.
Why deleted? It has been explicitly newly introduced! If it is currently not used/referenced ... we rather have to update our FTAs and reference it ;)
75635aa to
8027902
Compare
8027902 to
bc11a30
Compare
Requested changes were made and verified by @crimson11
Issue: SWP-276866