JosiahWI commented on code in PR #13684:
URL: https://github.com/apache/trafficserver/pull/13684#discussion_r4017786633
##########
src/proxy/ControlMatcher.cc:
##########
@@ -424,9 +424,13 @@ RegexMatcher<Data, MatchResult>::NewEntry(matcher_line
*line_info)
ink_assert(line_info->dest_entry < MATCHER_MAX_TOKENS);
ink_assert(pattern != nullptr);
- // Create the compiled regular expression
- regex_array[num_el].compile(pattern, error_msg, erroffset);
- if (regex_array[num_el].empty()) {
+ // Create the compiled regular expression. Check what compile() returned
rather than
+ // asking the object whether it holds a pattern: this slot is reused when a
previous
+ // line's Data::Init() failed, because that path leaves num_el where it was,
so the
+ // Regex here can still hold the pattern that line compiled. empty() would
then be
+ // false for a line whose own pattern never compiled, and the entry would be
accepted
+ // and matched against the earlier pattern under this line's configuration.
Review Comment:
I'm not convinced this level of comment depth is warranted for this
self-explanatory line of code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
