Fix string termination in BadPattern exception message #4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, we would overallocate the string passed back to the OCaml runtime containing the PCRE2 error message; now, we copy the raw error to an intermediate buffer on the stack before allocating an OCaml string of exactly the proper length. This change both fixes the absence of the string terminator and consumes less memory overall. Additionally, I've increased the size of the error message buffer to 256; while 128 is sufficient for the current set of errors supported by PCRE2, the maximum length of any of which is 92 including the NUL terminator, 256 gives us some breathing room should longer messages begin to be used. This length is also a form of unofficial standard for PCRE error message buffer sizes, being used in the Julia runtime and other high-profile open-source applications. Since the OCaml string we allocate will be exactly the length of the actual error message, the impact this additional stack allocation has is negligible.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following largely the same approach as #1, this patch fixes the improper string termination formerly performed for
BadPatternexception messages, which are sourced frompcre2_get_error_message(). It also increases the size of the buffer into which these error messages are copied, choosing the same constant of256as used by most other open-source projects (as found by a quick survey of uses of this function on GitHub): currently, the maximum length of any of PCRE2's error messages is 92 bytes including the null terminator, so this change moves us well out of the range of our buffer being too small.Prior to this patch:
With this patch: