💬 hebasto commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2362554073)
This should work:
```suggestion
std::optional<size_t> GetTotalRAM()
{
auto clamp{[](uint64_t v) { return size_t(std::min(v, uint64_t{std::numeric_limits<size_t>::max()})); }};
#ifdef WIN32
if (MEMORYSTATUSEX m{}; (m.dwLength = sizeof(m), GlobalMemoryStatusEx(&m))) return clamp(m.ullTotalPhys);
#else
if (long p{sysconf(_SC_PHYS_PAGES)}, s{sysconf(_SC_PAGESIZE)}; p > 0 && s > 0) return clamp(1ULL * p * s);
#endif
return std::nullopt;
}

```

See CI outputs for:
- Fr
...

💬 vasild commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2362610449)
I came here for the same reason ;-) Here is the patch I devised:

```diff
std::optional<size_t> GetTotalRAM()
{
- auto clamp{[](uint64_t v) { return size_t(std::min(v, uint64_t{std::numeric_limits<size_t>::max()})); }};
+ [[maybe_unused]] auto clamp{[](uint64_t v) { return size_t(std::min(v, uint64_t{std::numeric_limits<size_t>::max()})); }};
#ifdef WIN32
if (MEMORYSTATUSEX m{}; (m.dwLength = sizeof(m), GlobalMemoryStatusEx(&m))) return clamp(m.ullTotalPhys);
-#elif define
...

🤔 hebasto reviewed a pull request: "system: silence unused variable warning and make GetTotalRAM() work on FreeBSD"
(https://github.com/bitcoin/bitcoin/pull/33435#pullrequestreview-3247240250)
Additionally, I suggest guarding this check:https://github.com/bitcoin/bitcoin/blob/56c6daa64f6bc29fb83f3ec8940c37ddc549edeb/src/test/system_tests.cpp#L23 with the same `#ifdef`s as in the `GetTotalRAM()` implementation. Otherwise, this test will fail on any unmentioned system.

🤔 hodlinator reviewed a pull request: "node: Add --debug_runs/-waitfordebugger + --debug_cmd"
(https://github.com/bitcoin/bitcoin/pull/31723#pullrequestreview-3274687166)
Thanks for reviewing @l0rinc!

> Since we're editing production code for this I would prefer having a test which checks that `support was not included as WAIT_FOR_DEBUGGER` is logged, proving that we're correctly excluding it from code (it's fine if the test isn't passing in debug mode)

Feel it would be a bit performative since `#ifdef`s and default-`OFF` CMake variables are pretty cut & dry. Maybe there's some scenario I haven't thought of.

💬 hodlinator commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2387995922)
I'm concerned with adding an extra build permutation for CI. Maybe could turn it on in some debug build. Hm..

Changed C++ code to use `constexpr` over `#ifdef` so that the code at least gets regularly compiled.

💬 l0rinc commented on pull request "random: clarify where the environ extern is needed":
(https://github.com/bitcoin/bitcoin/pull/33714#discussion_r2466889461)
If it's only needed on Mac, wouldn't:
```suggestion
#ifdef __APPLE__
```
be better here?