Catch and avoid using CHIPMem.h uninitialized #2918
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
#### Problem Some code calls `Platform::Memory…()` allocation functions without having first called `Platform::MemoryInit()`. This is easy to miss because it silently works for the `CHIP_CONFIG_MEMORY_MGMT_MALLOC` configuration. #### Summary of Changes - In debug builds, have `Platform::Memory…()` verify that `Platform::MemoryInit()` has been called. - Fix some existing tests to call `Platform::MemoryInit()`. fixes project-chip#2899
andy31415
approved these changes
Sep 29, 2020
pullapprove
bot
requested review from
BroderickCarlin,
bzbarsky-apple,
chrisdecenzo,
hawk248,
jelderton,
rwalker-apple and
saurabhst
rwalker-apple
approved these changes
Sep 29, 2020
mspang
reviewed
Sep 29, 2020
src/lib/support/CHIPMem-Malloc.cpp
Outdated
|
|
||
| #else | ||
|
|
||
| static int memoryInitialized = 0; |
There was a problem hiding this comment.
Shouldn't this be atomic in builds w/ locking?
There was a problem hiding this comment.
Yes, good point. And should also check against calling Init while Init'd and Shutdown while shut down.
|
Possibly a good use for the "nifty counter" (AKA "Schwarz counter") idiom for initializing static objects before first use (as used by iostream to initialize cout)? https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Nifty_Counter |
BroderickCarlin
approved these changes
Sep 30, 2020
|
@kpschoedel can you rebase? should be good to go after that! |
mspang
approved these changes
Oct 1, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Some code calls
Platform::Memory…()allocation functions withouthaving first called
Platform::MemoryInit(). This is easy to miss becauseit silently works for the
CHIP_CONFIG_MEMORY_MGMT_MALLOCconfiguration.Summary of Changes
In debug builds, have
Platform::Memory…()verify thatPlatform::MemoryInit()has been called.Fix some existing tests to call
Platform::MemoryInit().fixes #2899 CHIPMem.h is sometimes used uninitialized