Skip to content
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

Fix libpcre2 bindings function pointers #13090

Conversation

straight-shoota
Copy link
Member

The function binding for pcre2_general_context_create receives two function pointers.
They're typed as void pointers instead of a proc type.
The problem is that at the call site the argument type of one of the callback functions is incorrectly declared as LibC::Int, but it should actually be LibC::SizeT. The function definition mentioned the expected callback type, but it also incorrectly used Int.
This puts it in the same error class as #13088.

It sometimes worked - including on all platforms where the original PCRE2 implementation was tested, so it wasn't noticed.

Crystal's lib bindings actually has special syntax for function pointers as detailed in https://crystal-lang.org/reference/1.7/syntax_and_semantics/c_bindings/callbacks.html

Function pointers can be expressed as Proc type. And when using proc literals at call site, you can even skip the argument types because they're inferred from the function binding.
So yay, with this patch the code is safer and more concise.

@straight-shoota straight-shoota added this to the 1.8.0 milestone Feb 19, 2023
@straight-shoota straight-shoota changed the base branch from master to fix/pcre2-meta February 22, 2023 07:36
@straight-shoota straight-shoota merged commit 9782ce7 into crystal-lang:fix/pcre2-meta Feb 22, 2023
@straight-shoota straight-shoota deleted the fix/libpcre2-function-pointer branch February 22, 2023 08:36
@straight-shoota straight-shoota modified the milestones: 1.8.0, 1.7.3 Mar 2, 2023
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants