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

Remove dead code in darwinssl for 10.6 support #1028

Closed

Conversation

danielgustafsson
Copy link
Member

The block supporting macOS 10.6 in CopyIdentityWithLabel() was duplicated with one copy sitting inside the block only covering 10.7+ and iOS. Remove the block while leaving the live one in place. Passes make test in a build with --enable-darwinssl --enable-debug.

@jay
Copy link
Member

jay commented Sep 19, 2016

#define CURL_SUPPORT_MAC_10_6 MAC_OS_X_VERSION_MIN_REQUIRED <= 1060
#define CURL_SUPPORT_MAC_10_7 MAC_OS_X_VERSION_MIN_REQUIRED <= 1070

My read of this is code guarded by CURL_SUPPORT_MAC_10_7 | CURL_BUILD_IOS doesn't mean the CURL_SUPPORT_MAC_10_6 is "dead" in that block.

This #endif ends the 10_6 block nested inside the 10_7 block, not
the 10_7 block.
@danielgustafsson
Copy link
Member Author

Yes, I stand corrected - I managed to confuse myself of the IOS part of the guard.

I think the comment on the #endif /* CURL_SUPPORT_MAC_10_7 */ line should read /* CURL_SUPPORT_MAC_10_7 */ though as in the new force pushed commit.

Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a correct fix indeed

@jay jay closed this in 79607ee Sep 19, 2016
@jay
Copy link
Member

jay commented Sep 19, 2016

Thanks

@danielgustafsson
Copy link
Member Author

Thanks for review and for applying.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants