x509: make Windows policy parameter type version-specific #286
Conversation
This file contains 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
Go 1.11 needs syscall.CertChainPolicyPara.ExtraPolicyPara to have type syscall.Pointer, but in previous versions of Go this had type uintptr. As we have a fork of crypto/x509, our source code for x509 can be a different version than the current compiler. To allow our code to work with both 1.11 and earlier versions, encapsulate the cast into a version-specific function. Fixes google#284
Codecov Report
@@ Coverage Diff @@
## master #286 +/- ##
=======================================
Coverage 69.98% 69.98%
=======================================
Files 68 68
Lines 8769 8769
=======================================
Hits 6137 6137
Misses 2086 2086
Partials 546 546Continue to review full report at Codecov.
|
kolyshkin
reviewed
Jul 4, 2018
| // syscall.CertChainPolicyPara is of type syscall.Pointer. See: | ||
| // https://github.com/golang/go/commit/4869ec00e87ef | ||
|
|
||
| func convertToPolicyParaType(p unsafe.Pointer) syscall.Pointer { |
There was a problem hiding this comment.
Perhaps we can do something like this, hiding all the complexity inside this function here:
func getCertChainPolicyPara(p *syscall.SSLExtraCertChainPolicyPara) *syscall.CertChainPolicyPara {
return &syscall.CertChainPolicyPara{
ExtraPolicyPara: (syscall.Pointer)(unsafe.Pointer(p))
}and simplify the main code:
para := getCertChainPolicyPara(sslPara)There was a problem hiding this comment.
Simplifying the main code isn't necessarily something we want -- we (mostly) try to minimize deltas from the upstream codebase so that it's easier to merge in new versions.
kolyshkin
reviewed
Jul 4, 2018
| // syscall.CertChainPolicyPara was of type uintptr. See: | ||
| // https://github.com/golang/go/commit/4869ec00e87ef | ||
|
|
||
| func convertToPolicyParaType(p unsafe.Pointer) uintptr { |
There was a problem hiding this comment.
similar code here:
func getCertChainPolicyPara(p *syscall.SSLExtraCertChainPolicyPara) *syscall.CertChainPolicyPara {
return &syscall.CertChainPolicyPara{
ExtraPolicyPara: (uintptr)(unsafe.Pointer(p))
}
Martin2112
approved these changes
Jul 5, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Go 1.11 needs syscall.CertChainPolicyPara.ExtraPolicyPara to
have type syscall.Pointer, but in previous versions of Go this
had type uintptr.
As we have a fork of crypto/x509, our source code for x509 can be
a different version than the current compiler.
To allow our code to work with both 1.11 and earlier versions,
encapsulate the cast into a version-specific function.
Fixes #284