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

cmake WITH_LIBSODIUM option is broken #2349

Merged
merged 3 commits into from
Feb 22, 2017
Merged

cmake WITH_LIBSODIUM option is broken #2349

merged 3 commits into from
Feb 22, 2017

Conversation

boringuy
Copy link
Contributor

@boringuy boringuy commented Feb 22, 2017

  • Fixed the variable name in platform.hpp.in

- Fixed the variable name in platform.hpp.in
- Fixed #if check for randombytes_close() when libsodium is used
src/ctx.cpp Outdated
@@ -133,7 +133,7 @@ zmq::ctx_t::~ctx_t ()

// If we've done any Curve encryption, we may have a file handle
// to /dev/urandom open that needs to be cleaned up.
#ifdef ZMQ_HAVE_CURVE
#if defined (ZMQ_USE_TWEETNACL)
Copy link
Member

Choose a reason for hiding this comment

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

are you sure about this? sodium has this function too
https://download.libsodium.org/doc/generating_random_data/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah... I fixed this before I fix the CMAKE variable then. So, this error was cause by the same problem. I will change this back. Thanks.

@boringuy
Copy link
Contributor Author

Ok. Fixed the pull request, it ends up to be just a one line change in platform.hpp.in

@bluca bluca merged commit d6f4263 into zeromq:master Feb 22, 2017
@bluca
Copy link
Member

bluca commented Feb 22, 2017

Thanks! For your next contributions, if you have fixup commits, please rebase and squash them into the original one.

Also please follow the guidelines set in the PR template for the commit message:

Problem: x is broken

Solution: do y and z to fix it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants