Fix logic of memory_cleanse() on MSVC and clean up docs
Summary:
https://github.com/bitcoin/bitcoin/pull/16158/commits/cac30a436cab3641bba3b774d3d3ddbc426e7908
Commit fbf327b13868861c2877c5754caf5a9816f2603c ("Minimal code
changes to allow msvc compilation.") was indeed minimal in terms
of lines touched. But as a result of that minimalism it changed the
logic in memory_cleanse() to first call std::memset() and then
additionally the MSVC-specific SecureZeroMemory() function, and it
also moved a comment to the wrong location.
This commit removes the superfluous call to std::memset() on MSVC
and ensures that the comment is in the right position again.
https://github.com/bitcoin/bitcoin/pull/16158/commits/f53a70ce95231d34bb14cd6c58503927e8d7ff59
So far, the documentation of memory_cleanse() is a verbatim copy of
the commit message in BoringSSL, where this code was originally
written. However, our code evolved since then, and the commit message
is not particularly helpful in the code but is rather of historical
interested in BoringSSL only.This commit improves improves the comments around memory_cleanse()
and gives a better rationale for the method that we use. This commit
touches only comments.
This is a backport of Core PR16158
Test Plan: ninja all check-all
Reviewers: #bitcoin_abc, majcosta
Reviewed By: #bitcoin_abc, majcosta
Subscribers: majcosta, Fabien
Differential Revision: https://reviews.bitcoinabc.org/D8375