Update crypto_argon2.3monocypher - fixes errors and warnings that can lead to the incorrect solution #265
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
… lead to incorrect solution Closes: LoupVaillant#264 Fixes these issues, by allowing password array to autosize, and then making sure to drop the \0 character when determining the size of the string, and also reorders the crypto_argon2_inputs field designators to remove a warning. ``` main.cpp:83:24: error: initializer-string for char array is too long, array size is 14 but initializer has size 15 (including the null terminating character) uint8_t password[14] = "Okay Password!"; ^~~~~~~~~~~~~~~~ main.cpp:87:5: warning: ISO C++ requires field designators to be specified in declaration order; field 'pass_size' will be initialized after field 'salt' [-Wreorder-init-list] .salt = salt, /* Salt for the password */ ^~~~~~~~~~~~~~~~~ ```
|
Looks pretty good to me, thanks for the fix! This may help more than you realise, such issues not only waste time (sorry about yours), they also tend to hurt whatever trust users may have in the library. |
|
@LoupVaillant Cool, glad it helps! 😄 Appreciate your work, and glad this exists! 👍 |
|
Realizing now that I was conflating strlen with sizeof! 🤦 strlen would be a fine replacement for sizeof here that would avoid all confusion. |
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.
Closes: #264
Fixes these issues, by allowing password array to autosize, and then making sure to drop the \0 character when determining the size of the string, and also reorders the crypto_argon2_inputs field designators to remove a warning.