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: Use new prototypes for objc_msgSend #432

Merged
merged 2 commits into from
Jan 22, 2021

Conversation

rajivshah3
Copy link
Contributor

The use of objc_msgSend with OBJC_OLD_DISPATCH_PROTOTYPES is dangerous. The new prototypes were introduced specifically to prevent odd behavior that may occur as a result of how variadic parameters are handled (see this article). This PR updates each call to objc_msgSend so that it is casted appropriately for the corresponding Objective-C call.

@nothingismagick
Copy link
Contributor

Great catch @rajivshah3 - but we need to verify that its really working equivalently. Anyone care to test this on MacOS?

@wusyong
Copy link

wusyong commented Jul 24, 2020

I can take some time test this tonight. Might also test if we can increase the readability a bit.

@wusyong
Copy link

wusyong commented Jul 24, 2020

LGTM. I tested some macro on it but they become worse. Guess it's better to leave as is.

@wusyong
Copy link

wusyong commented Jan 22, 2021

New macbook seems to enable "Strict Checking of objc_msgSend Calls" by default.
And this PR can resolve it. webview_rust#51 is also waiting on this.
@rajivshah3 could you rebase the PR when you have some time? And maybe someone who has permission could merge it.

@rajivshah3 rajivshah3 marked this pull request as ready for review January 22, 2021 06:13
@wusyong wusyong merged commit 7b1b723 into webview:master Jan 22, 2021
@rajivshah3 rajivshah3 deleted the fix/new-objc-prototypes branch February 8, 2021 19:21
@SteffenL SteffenL added this to the v0.10.0 milestone Sep 1, 2023
@SteffenL SteffenL added the enhancement New feature, enhancement of an existing feature or a request label Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature, enhancement of an existing feature or a request
Development

Successfully merging this pull request may close these issues.

None yet

4 participants