differential-privacy
differential-privacy copied to clipboard
Improving comments in privacy-critical code blocks.
The lack of comments makes this code harder to audit. Consider https://github.com/google/differential-privacy/blob/master/differential_privacy/algorithms/rand.cc
It is not clear from reading the code what SecureURBG::RefreshCache
does. Is it that you are computing a whole buffer of randomness at once and then feeding it out byte-by-byte? That is not a cache, then, but a buffer.
In SecureURBG::result_type SecureURBG::operator
, it is not clear why old_index
is copied from current_index
before the memcpy
operation:
SecureURBG::result_type SecureURBG::operator()() {
absl::WriterMutexLock lock(&mutex_);
if (current_index_ + sizeof(result_type) > kCacheSize) {
RefreshCache();
}
int old_index = current_index_;
current_index_ += sizeof(result_type);
result_type result;
std::memcpy(&result, cache_ + old_index, sizeof(result_type));
return result;
}
Why the extra copy, and not simply copy it like this:
SecureURBG::result_type SecureURBG::operator()() {
absl::WriterMutexLock lock(&mutex_);
if (current_index_ + sizeof(result_type) > kCacheSize) {
RefreshCache();
}
result_type result;
std::memcpy(&result, cache_ + current_index_, sizeof(result_type));
current_index_ += sizeof(result_type);
return result;
}
The programmer's intent is not clear from the C++ code, so it would be useful to have comments to explain it. It would also be nice to know what URBG stands for.