💬 vasild commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1604635967)
Here is a standalone program to get the default gateway using a netlink socket:

<details>
<summary>netlink_get_default_route.cc</summary>

```cc
#include <arpa/inet.h>
#include <assert.h>
#include <errno.h>
#include <net/if.h>
#include <netinet/in.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <unistd.h>

#ifdef __linux__
#include <linux/rtnetlink.h>
#elif defined(__FreeBSD__)
#include <netlink/netlink.h
...

💬 hebasto commented on pull request "build: add `-Wundef`":
(https://github.com/bitcoin/bitcoin/pull/29876#issuecomment-2157962927)
> We'll probably need to fixup the failing ARM and previous releases job upstream:
>
> ```shell
> In file included from minisketch/src/fields/generic_common_impl.h:12,
> from minisketch/src/fields/generic_4bytes.cpp:12:
> minisketch/src/fields/../int_utils.h:162:7: error: "HAVE_CLZ" is not defined, evaluates to 0 [-Werror=undef]
> 162 | #elif HAVE_CLZ
> | ^~~~~~~~
> ```

The upstream issue has been addressed in https://github.com/sipa/minisketch/pull/88.
...

💬 theuni commented on pull request "build, ci: Fix linking `bitcoin-chainstate.exe` to `bitcoinkernel.dll` on Windows":
(https://github.com/bitcoin/bitcoin/pull/31158#issuecomment-2637937419)
> > Please give https://github.com/theuni/bitcoin/commits/pr31158/ a shot. It's not meant to be robust, but it should illustrate the fix (using dllimport for bitcoin-chainstate.exe).
>
> CI fails: https://github.com/theuni/bitcoin/actions/runs/13163570761/job/36738155052

Yeah, the above is only enough to test `bitcoin-chainstate.exe`. In reality, the condition in the header would need to be something like:
`#elif defined(MSC_VER) && !defined(STATIC_LIBBITCOINKERNEL) && !defined(BITCOIN_IN
...

💬 theuni commented on pull request "build, ci: Fix linking `bitcoin-chainstate.exe` to `bitcoinkernel.dll` on Windows":
(https://github.com/bitcoin/bitcoin/pull/31158#discussion_r1946935711)
The meaning of `BITCOINKERNEL_STATIC` here is overloaded. Just to differentiate the use-cases, I'd prefer to have a 3rd define here:

`#elif defined(_WIN32) && !defined(BITCOIN_BUILD_INTERNAL) && !defined(BITCOINKERNEL_STATIC)`

`BITCOINKERNEL_STATIC`: I'm a user of the kernel building against a static kernel.
`BITCOIN_BUILD_INTERNAL`: I'm a component of Core who's not building against the kernel at all, I just happen to be seeing this header.

The former should stay as-is for bitcoin-cha
...

💬 edwargix commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2285678340)
please forgive me if I'm misunderstanding this, but can't this logic be simplified to:
```suggestion
#if defined(_WIN32)
#define BITCOINKERNEL_API __declspec(dllexport)
#elif defined(__GNUC__)
#define BITCOINKERNEL_API __attribute__((visibility("default")))
```

💬 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
...

📝 vasild opened a pull request: "system: silence unused variable warning and make GetTotalRAM() work on FreeBSD"
(https://github.com/bitcoin/bitcoin/pull/33435)
This patch achieves two things:
1. Fix unused variable warning (https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2362493046)
2. Enable `GetTotalRAM()` on FreeBSD. The exact same code works and produces correct results on FreeBSD, so just extend the `#elif` condition.

Prior discussion: https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2362493046

💬 hebasto commented on pull request "system: silence unused variable warning and make GetTotalRAM() work on FreeBSD":
(https://github.com/bitcoin/bitcoin/pull/33435#discussion_r2362917905)
```suggestion
#elif defined(__linux__) || defined(__APPLE__) || defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__NetBSD__) || defined(__illumos__)
```

💬 l0rinc commented on pull request "system: silence unused variable warning and make GetTotalRAM() work on FreeBSD":
(https://github.com/bitcoin/bitcoin/pull/33435#discussion_r2364461162)
Based on https://github.com/bitcoin/bitcoin/blob/56c6daa64f6bc29fb83f3ec8940c37ddc549edeb/src/common/system.cpp#L79 I also suggest something similar:
```suggestion
#elif defined(__linux__) || defined(__APPLE__) || defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__NetBSD__)
```