mwcc_orderedhashmap.t: limit cycles number for memory sanitizer #338
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
Signed-off-by: Aleksandr Ivanov <[email protected]>
Signed-off-by: Aleksandr Ivanov <[email protected]>
Signed-off-by: Aleksandr Ivanov <[email protected]>
Signed-off-by: Aleksandr Ivanov <[email protected]>
alexander-e1off
changed the title
678098
requested changes
Jun 27, 2024
There was a problem hiding this comment.
- Let's make the code block defining
k_NUM_ELEMENTSidentical for 2 UTs. There is no need to keep these 2 different inverted conditions. - It is not enough to just check if
__has_featureis defined. This macro can exist but all sanitizers might be disabled. - I think we should still check for
__SANITIZE_MEMORY__aka GCC-supported macro. https://github.com/torvalds/linux/blob/afcd48134c58d6af45fb3fdb648f1260b20f2326/include/linux/compiler-clang.h#L59 - Overall, I think it's not necessary to try to fit everything in one
ifcondition. You can make many branches for each specific case with very basic conditions for each branch. It will be more code lines, but it's more maintainable than one complex condition. - Not necessary to put
__has_feature(memory_sanitizer)check to the preprocessor#if, you can use a ternary assignment fork_NUM_ELEMENTSinstead
Signed-off-by: Aleksandr Ivanov <[email protected]>
Signed-off-by: Aleksandr Ivanov <[email protected]>
Signed-off-by: Aleksandr Ivanov <[email protected]>
|
Addressed review comments |
Signed-off-by: Aleksandr Ivanov <[email protected]>
Signed-off-by: Aleksandr Ivanov <[email protected]>
lukedigiovanna
pushed a commit
to lukedigiovanna/blazingmq
that referenced
this pull request
Jul 2, 2024
alexander-e1off
added a commit
to alexander-e1off/blazingmq
that referenced
this pull request
Oct 24, 2024
alexander-e1off
added a commit
to alexander-e1off/blazingmq
that referenced
this pull request
Oct 24, 2024
alexander-e1off
added a commit
to alexander-e1off/blazingmq
that referenced
this pull request
Oct 24, 2024
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.
No description provided.