Correctly initialize defaulted external sequence #1956
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
An external mutable struct containing a sequence member that for which no value is provided by the CDR was not initialized correctly: only the length field was set to 0, not the other fields. If the _release field happens to be false, this is harmless; if not it results in freeing of unallocated memory. Signed-off-by: Erik Boasson <eb@ilities.com>
dpotman
reviewed
Mar 28, 2024
| { | ||
| seq->_buffer = NULL; | ||
| seq->_maximum = 0; | ||
| seq->_release = true; |
There was a problem hiding this comment.
I think this could be set to false. Seems that this could result in freeing a NULL pointer in src/core/cdr/src/dds_cdrstream.c:3283, but that won't do any harm in current allocators.
There was a problem hiding this comment.
At first I wrote initialization code at line 1958 (or thereabouts) and wondered about how to initialise _release. Then I realised I could refactor things and have a single implementation. The true is simply what it used to do. Therefore I am tempted to leave it as-is in this PR.
dpotman
approved these changes
Mar 28, 2024
This test relies on the combination of the address sanitizer and the undefined behaviour sanitizer: the former for initializing freshly allocated memory, the latter for faulting on trying to load `bool` from memory with a value other than 0 or 1. Signed-off-by: Erik Boasson <eb@ilities.com>
eboasson
force-pushed
the
fix-default-seq-init
branch
from
c7e7f15
to
f671ec3
Compare
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.
An external mutable struct containing a sequence member that for which no value is
provided by the CDR was not initialized correctly: only the length field was set to 0, not
the other fields. If the _release field happens to be false, this is harmless; if not it
results in freeing of unallocated memory.