Skip to content

Commit 62af467

Browse files
committed
Merge #20082: [bugfix] random: fixes read buffer to use min rather than max
bd52151 random: fixes read buffer resizing in RandAddSeedPerfmon (Ethan Heilman) Pull request description: As shown below when resizing the read buffer `vData` `std::max((vData.size() * 3) / 2, nMaxSize)` is used. This means that the buffer size immediately jumps to `nMaxSize`. I believe the intend of this code is to grow the buffer size through several steps rather than immediately resize it to the max size. ```cpp std::vector<unsigned char> vData(250000, 0); long ret = 0; unsigned long nSize = 0; const size_t nMaxSize = 10000000; // Bail out at more than 10MB of performance data while (true) { nSize = vData.size(); ret = RegQueryValueExA(HKEY_PERFORMANCE_DATA, "Global", nullptr, nullptr, vData.data(), &nSize); if (ret != ERROR_MORE_DATA || vData.size() >= nMaxSize) break; vData.resize(std::max((vData.size() * 3) / 2, nMaxSize)); // Grow size of buffer exponentially } ``` vData always starts at size 250,000 and nMaxSize is always 10,000,000 so the first time this line is reached: ```cpp vData.resize(std::max((vData.size() * 3) / 2, nMaxSize)); ``` the effect will always be to resize vData to nMaxSize. Then because the loop terminates when vData.size >= 10,000,000 only one resize operation will take place. To fix this issue we replace `std::min` with `std::max` This PR also adds a comment clarifying the behavior of this function the first time it is called. ACKs for top commit: fanquake: ACK bd52151 - thanks for taking a look at this Ethan. Swapping from `std::max` to `std::min` here certainly seems correct. Tree-SHA512: 7c65f700e5bbe44bc2f1ffdcdc99ec19c542894c95b5ee9791facd09d02afae88d1f8f35af129719e4860db94bc790856e7adb1d218a395381e7c2913b95f1d0
2 parents a1e0359 + bd52151 commit 62af467

File tree

1 file changed

+3
-2
lines changed

1 file changed

+3
-2
lines changed

src/randomenv.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ void RandAddSeedPerfmon(CSHA512& hasher)
6767
#ifdef WIN32
6868
// Seed with the entire set of perfmon data
6969

70-
// This can take up to 2 seconds, so only do it every 10 minutes
70+
// This can take up to 2 seconds, so only do it every 10 minutes.
71+
// Initialize last_perfmon to 0 seconds, we don't skip the first call.
7172
static std::atomic<std::chrono::seconds> last_perfmon{std::chrono::seconds{0}};
7273
auto last_time = last_perfmon.load();
7374
auto current_time = GetTime<std::chrono::seconds>();
@@ -83,7 +84,7 @@ void RandAddSeedPerfmon(CSHA512& hasher)
8384
ret = RegQueryValueExA(HKEY_PERFORMANCE_DATA, "Global", nullptr, nullptr, vData.data(), &nSize);
8485
if (ret != ERROR_MORE_DATA || vData.size() >= nMaxSize)
8586
break;
86-
vData.resize(std::max((vData.size() * 3) / 2, nMaxSize)); // Grow size of buffer exponentially
87+
vData.resize(std::min((vData.size() * 3) / 2, nMaxSize)); // Grow size of buffer exponentially
8788
}
8889
RegCloseKey(HKEY_PERFORMANCE_DATA);
8990
if (ret == ERROR_SUCCESS) {

0 commit comments

Comments
 (0)