-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix dangling reference in parser::Parse with unique_ptr #8
Conversation
Currently the tl;dr: the returned |
include/inja/parser.hpp
Outdated
Template parse(std::string_view input, std::string_view path) { | ||
auto result = Template(static_cast<std::string>(input)); | ||
parse_into(result, path); | ||
std::shared_ptr<Template> parse(std::string_view input, std::string_view path) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we use unique_ptr here? i don't see this shared anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great callout, thanks! Made that change
@@ -105,7 +105,7 @@ class Environment { | |||
|
|||
Template parse(std::string_view input) { | |||
Parser parser(parser_config, lexer_config, template_storage, function_storage); | |||
return parser.parse(input, input_path); | |||
return *parser.parse(input, input_path); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are doing parse just on config time, right? if so i'm ok with this copy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we are only parsing at config time. I will add a comment to the area where this is called in envoy-gloo to explicitly call out that we should only be parsing at config time.
The variable
result
was a dangling reference when returning fromParser::parse
function due to internals ofparse_into
, so we create astd::unique_ptr
of the template and return that instead.This is to resolve the static analysis warning generated in our envoy repos: