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 atfork flag. Avoid double-negatives :) #3815

Closed
wants to merge 1 commit into from
Closed

Fix atfork flag. Avoid double-negatives :) #3815

wants to merge 1 commit into from

Conversation

richsalz
Copy link
Contributor

Alternate fix for #3814

@richsalz richsalz added branch: master Merge to master branch approval: review pending This pull request needs review by a committer labels Jun 30, 2017
@richsalz richsalz self-assigned this Jun 30, 2017
@richsalz
Copy link
Contributor Author

ping @bernd-edlinger

@bernd-edlinger
Copy link
Member

okay wil check if it works.

@bernd-edlinger
Copy link
Member

your patch works,
but why not simply initialize the at-fork thing when OPENSSL_fork_prepare is called
for the first time?

@richsalz
Copy link
Contributor Author

richsalz commented Jun 30, 2017 via email

@bernd-edlinger
Copy link
Member

Fine for me if it's OK for you.

@richsalz
Copy link
Contributor Author

thanks!

@richsalz richsalz closed this Jun 30, 2017
@richsalz richsalz deleted the fix-atfork branch June 30, 2017 18:50
levitte pushed a commit that referenced this pull request Jun 30, 2017
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from #3815)
@mattcaswell
Copy link
Member

I think most programs will not fork, and those that do need to add a flag. Not sure if that’s the best way to go, but at least it’s simplest to understand and explain.

Not sure why that is simplest. Surely if we just do this by default then most people can forget about it? In some scenarios the code calling us does not even know if a fork will take place or not (e.g. consider a shared library that links to OpenSSL, where the end application does the fork).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: review pending This pull request needs review by a committer branch: master Merge to master branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants