[1.2] ReplacemenProductListManager is assuming static memory allocation #28148
Assignees
Labels
Comments
|
@cliffamzn Also, see #28095 (review) |
cliffamzn
added a commit
to cliffamzn/connectedhomeip
that referenced
this issue
Jul 21, 2023
Addressing feedback from PR project-chip#28095 and also addressing project-chip#28148
cliffamzn
added a commit
to cliffamzn/connectedhomeip
that referenced
this issue
Jul 21, 2023
Addressing feedback from PR project-chip#28095 and also addressing project-chip#28148 Tested to ensure functionality still works as expected using the all-clusters-app as well as the resource-monitoring-app
|
@bzbarsky-apple - included suggestions from previous changes in the latest PR as well. |
cliffamzn
added a commit
that referenced
this issue
Jul 27, 2023
* Modifying ReplacementProductListManager to be generic Addressing feedback from PR #28095 and also addressing #28148 This change simplifies the implementation of the ReplacementProductList and also introduces a DynamicReplacementProductList example which better represents devices fetching this product list from flash. It includes some simplifications to the structure of the code and a few additional unit tests to exercise the behavior. Tested to ensure functionality still works as expected using the all-clusters-app as well as the resource-monitoring-app
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reproduction steps
The API looks like this:
But that struct has a CharSpan inside, and how does the callee know when it's safe to free the memory the CharSpan is pointing to? It doesn't.
We need to stop using "static stick it all in memory" implementations to back our delegate APIs, unless we really expect all applications to do it that way....
@cliffamzn @tcarmelveilleux @chrisdecenzo
Bug prevalence
Always
GitHub hash of the SDK that was being used
d2d4701
Platform
core
Platform Version(s)
No response
Type
Common Cluster Logic
Anything else?
@cliffamzn I suggest doing one of two things. Either have separate getters for the separate parts of the struct, or have a struct type that includes storage so that you can put one on the stack and the callee can copy the data into it. The latter is what I had been suggesting you do on Slack, if you recall: inherit from one of the generated structs, pull in any method definitions, adapt your subclass for your needs...
The text was updated successfully, but these errors were encountered: