Copilot commented on code in PR #13683:
URL: https://github.com/apache/trafficserver/pull/13683#discussion_r4052242010
##########
src/tsutil/Regex.cc:
##########
@@ -278,11 +343,16 @@ RegexMatchContext::operator=(RegexMatchContext const
&other)
{
if (&other != this) {
auto ptr = _MatchContext::get(other._match_context);
- if (nullptr != ptr) {
- pcre2_match_context *const ctx = pcre2_match_context_copy(ptr);
- _MatchContext::set(_match_context, ctx);
- } else {
- _MatchContext::set(_match_context, nullptr);
+
+ // Take the copy before releasing what this object already holds, so a
failing
+ // copy leaves it holding its old context rather than a freed one.
Releasing it
+ // is what this operator used to omit, and every assignment leaked one
context.
+ pcre2_match_context *const ctx = nullptr != ptr ?
pcre2_match_context_copy(ptr) : nullptr;
+ pcre2_match_context *const old = _MatchContext::get(_match_context);
+
+ _MatchContext::set(_match_context, ctx);
+ if (old != nullptr) {
+ pcre2_match_context_free(old);
}
Review Comment:
`pcre2_match_context_copy()` can return `nullptr` when it cannot allocate
memory. In that case this code still replaces the destination with `nullptr`
and frees `old`, so a transient copy failure discards the existing match limit
and JIT callback—the opposite of the comment's promised strong-failure
behavior. Only clear/free `old` when `ptr` was null, or when `ctx` is a
successful copy; otherwise leave the destination unchanged.
--
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]
