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.

Sign up for GitHub

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

Jump to bottom

Make default_method mostly compile-time #3146

Closed
wants to merge 1 commit into from
Closed

Make default_method mostly compile-time #3146

wants to merge 1 commit into from

Conversation

richsalz
Copy link
Contributor

@richsalz richsalz commented Apr 7, 2017

This is #2244 picked to 1.1.0
It keeps the RSA_null stuff.

@richsalz richsalz added the 1.1.0 label Apr 7, 2017
@richsalz richsalz self-assigned this Apr 7, 2017
@richsalz
Copy link
Contributor Author

ping @mattcaswell this is the 1.1.0 version of #2244 (which you had approved, hence the ping)

@mattcaswell
Copy link
Member

I'm slightly confused by your comment above "It keeps the RSA_null stuff" - as it doesn't seem to, i.e. the actual code for RSA_null_method() is retained, but it will never be used. Previously building with RSA_NULL caused RSA_null_method() to be used.

@richsalz
Copy link
Contributor Author

I meant it keeps the function.
It didn't work. We never tested it.

@geoffthorpe
Copy link
Contributor

Won't this alter behaviour on an existing/stable branch? E.g. could a wrapper script get borked by the change in semantics?

@richsalz
Copy link
Contributor Author

I don't believe so. The old code; we are narrowing the window of opportunity for borking. There is no change in semantics, rather than being lazy-init, it's now compile-time init.

@geoffthorpe
Copy link
Contributor

OK, so this is still the "default ENGINE == NULL" case? If so, I think it makes sense.

@richsalz
Copy link
Contributor Author

yes it is.

Make default_method mostly compile-time
56e60b1 
Document thread-safety issues

Cherry-pick from 076fc55 but
keeps the RSA_null method.
geoffthorpe
geoffthorpe approved these changes May 29, 2017
View reviewed changes
Copy link
Contributor

@geoffthorpe geoffthorpe left a comment

Choose a reason for hiding this comment

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

Looks good, especially now that the error codes aren't clobbered. :-)

@richsalz
Copy link
Contributor Author

:)

thanks.

@richsalz richsalz closed this May 29, 2017
@richsalz richsalz deleted the pr2244-for-110 branch May 29, 2017 23:15
levitte pushed a commit that referenced this pull request May 29, 2017
Make default_method mostly compile-time
b4d6f71 
Document thread-safety issues

Cherry-pick from 076fc55 but
keeps the RSA_null method.

Reviewed-by: Geoff Thorpe <geoff@openssl.org>
(Merged from #3146)
pracj3am pushed a commit to cdn77/openssl that referenced this pull request Aug 22, 2017
@pracj3am
Make default_method mostly compile-time
cfeb2d4 
Document thread-safety issues

Cherry-pick from 076fc55 but
keeps the RSA_null method.

Reviewed-by: Geoff Thorpe <geoff@openssl.org>
(Merged from openssl#3146)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geoffthorpe geoffthorpe geoffthorpe approved these changes

Assignees

@richsalz richsalz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@richsalz @mattcaswell @geoffthorpe