-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix: add PCRE2 capability to standalone module #3377
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
base: v2/master
Are you sure you want to change the base?
Conversation
|
||
pcre_fullinfo((const pcre *)preg->re_pcre, NULL, PCRE_INFO_CAPTURECOUNT, &nsub); | ||
preg->re_nsub = nsub; | ||
#endif // end of WITH_PCRE |
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.
#endif // end of WITH_PCRE | |
#endif |
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.
Instead of removing this comment, I should add a new comment to line 94:
#else // otherwise use PCRE
#ifdef PCRE_ERROR_BADUTF8_OFFSET | ||
case PCRE_ERROR_BADUTF8_OFFSET: return AP_REG_INVARG; | ||
#endif | ||
#endif // end of WITH_PCRE |
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.
#endif // end of WITH_PCRE | |
#endif |
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.
Same as here.
|
what
This PR adds
PCRE2
related code tostandalone
module.why
Previously if the user chose PCRE2 regex engine (with
--with-pcre2
at build time), the standalone module did not handle that, and still wanted to be built with the old regex engine.Furthermore, the modules
make test
workflow also usesstandalone
module too, and withPCRE2
the whole test case was broken. This patch fixes that too.