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

Allow to pass callbacks in varargs #885

Merged
merged 1 commit into from Feb 28, 2021
Merged

Allow to pass callbacks in varargs #885

merged 1 commit into from Feb 28, 2021

Conversation

vincentisambart
Copy link
Contributor

As I explained in #732 (comment), if you passed callbacks in varargs, a NULL pointer would be dereferenced and your program would crash.
I modified variadic_invoke to pass proper callbackParameters and callbackCount to rbffi_SetupCallParams, and that seems to fix the problem.

@vincentisambart
Copy link
Contributor Author

@larskanis Hi! Would you have anything against allowing to pass callbacks via varargs arguments?

@vincentisambart
Copy link
Contributor Author

By the way, passing callbacks in varargs might look like a rare use case, but functions of very used libraries like libcurl or sqlite use it.

For example, libcurl's curl_easy_getinfo function is declared as follows:

CURL_EXTERN CURLcode curl_easy_getinfo(CURL *curl, CURLINFO info, ...);

A library called ethon was declaring its arguments as [:pointer, :info, :pointer], even though the correct definition should be [:pointer, :info, :varargs].
That was working properly on many platforms, as using varargs or not had a similar ABI, but on ARM Macs, the ABI is different so that does not work properly. A bit more details in typhoeus/ethon#180.

The correct arguments definition needs this PR's fix to work properly.

@larskanis larskanis merged commit ba59d99 into ffi:master Feb 28, 2021
@larskanis
Copy link
Member

@vincentisambart Thank you for implementing this!

I merged your PR and did some fine tuning on the master branch. Let me know, if I misunderstood something! I'll try to make a new release soon, but I have only little spare time currently.

@vincentisambart vincentisambart deleted the pass-callback-in-varargs branch February 28, 2021 22:49
@vincentisambart
Copy link
Contributor Author

Thanks a lot!
Sorry for the fix you had to do. It's been too long since last time I wrote C code and completely forgot about freeing memory 😅
I tried the latest master branch with my use case and it still seems to work well 😄

No worries, I totally understand spare time is a limited resource.
Looking forward to the release 😁

@jdarnok
Copy link

jdarnok commented Mar 1, 2021

Thanks @vincentisambart and @larskanis for the great job you are doing here! one question regarding a release - can I know how long we have to wait for the release? there is quite a lot of people being blocked by it, including me 😄

This was referenced Mar 8, 2021
This was referenced Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants