Fix issue when using intermediate string returns #2048
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
In MSVC the following can lead to segfault: sass_copy_c_string(stream.str().c_str()); Reason is that the string returned by str() is disposed before sass_copy_c_string is invoked. The string is actually a stack object, so indeed nobody is holding on to it. So it seems perfectly fair to release it right away. So the const char* by c_str will point to invalid memory. I'm not sure if this is the behavior for all compiler, but I'm pretty sure we would have gotten more issues reported if that would be the case. Wrapping it in a functions seems the cleanest approach as the function must hold on to the stack variable until it's done.
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.
Fixes #2046
In MSVC the following can lead to segfault:
sass_copy_c_string(stream.str().c_str());Reason is that the string returned by
str()is disposed beforesass_copy_c_stringis invoked. The string is actually a stackobject, so indeed nobody is holding on to it. So it seems
perfectly fair to release it right away. So the
const char*by
c_strwill point to invalid memory. I'm not sure if this isthe behavior for all compiler, but I'm pretty sure we would
have gotten more issues reported if that would be the case.
Wrapping it in a function seems the cleanest approach as the
function must hold on to the stack variable until it's done.
Note. did a quick search for
str().c_str()and sanitized one other case.Seems we do not use this to often (good!). But worth to keep on our mind!
My gut feeling tells me that this is basically another incarnation of #1462