HomePhabricator

Fix logic of memory_cleanse() on MSVC and clean up docs

Description

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

Details

Provenance
Aaron Clauson <aaron@sipsorcery.com>Authored on Nov 9 2017, 20:06
PiRKCommitted on Nov 13 2020, 06:15
PiRKPushed on Nov 13 2020, 06:15
Reviewer
Restricted Project
Differential Revision
D8375: Fix logic of memory_cleanse() on MSVC and clean up docs
Parents
rABC15fc8dd009b4: doc: Include static members in Doxygen
Branches
Unknown
Tags
Unknown