JosiahWI commented on code in PR #13685:
URL: https://github.com/apache/trafficserver/pull/13685#discussion_r4017730487
##########
src/tsutil/Regex.cc:
##########
@@ -330,7 +330,26 @@ Regex::Regex(Regex const &other)
if (other_code != nullptr) {
// Use PCRE2's built-in function to deep copy the compiled pattern
auto *copied_code = pcre2_code_copy(other_code);
- _Code::set(_code, copied_code);
+
+ // pcre2_code_copy() returns null when it cannot obtain memory. Leave the
object empty
+ // in that case, which is the state a default constructed Regex is in and
which
+ // empty() reports truthfully, rather than compiling a null pattern.
+ if (copied_code != nullptr) {
+ // pcre2_code_copy() does not carry the machine code the JIT produced,
because that
+ // code is position dependent. Without this the copy would match on the
interpreter:
+ // same answers, much slower, and a different set of resource limits, so
a pattern
+ // that reports a JIT stack limit through the original would quietly
match through
+ // the copy. Compile it again, exactly as Regex::compile() does for a
new pattern.
+ //
+ // The result is not checked, for the same reason compile() does not
check it: a
+ // pattern the JIT will not take still matches correctly on the
interpreter, and this
+ // class has no way to tell a caller which engine it ended up with.
Whether a build
+ // even has a JIT is not one error code either, so a check here would
have to know
+ // three of them. Reporting the engine is what the replacement API adds.
Review Comment:
Is duplicated the whole comment from the unit test really worth it?
--
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]
