💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2002955096)
I'm not exactly sure how we got to the `min_alloc` of `9` on these platforms, but we can probably simplify the platform dependent code to:
```C++
#if defined(__arm__) || SIZE_MAX == UINT64_MAX
constexpr size_t min_alloc{9};
#else
constexpr size_t min_alloc{0};
#endif
```
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2002955096)
I'm not exactly sure how we got to the `min_alloc` of `9` on these platforms, but we can probably simplify the platform dependent code to:
```C++
#if defined(__arm__) || SIZE_MAX == UINT64_MAX
constexpr size_t min_alloc{9};
#else
constexpr size_t min_alloc{0};
#endif
```
💬 hebasto commented on pull request "build: simplify *ifaddr handling":
(https://github.com/bitcoin/bitcoin/pull/32446#discussion_r2079675289)
"Define to 1 if '*ifaddrs' are available" implies that "it remains undefined otherwise". To achieve this, I suggest:
```suggestion
/* Define to 1 if '*ifaddrs' are available. */
#cmakedefine HAVE_IFADDRS 1
```
This also allows to replace the following `#if HAVE_IFADDRS` with `#ifdef HAVE_IFADDRS`.
(https://github.com/bitcoin/bitcoin/pull/32446#discussion_r2079675289)
"Define to 1 if '*ifaddrs' are available" implies that "it remains undefined otherwise". To achieve this, I suggest:
```suggestion
/* Define to 1 if '*ifaddrs' are available. */
#cmakedefine HAVE_IFADDRS 1
```
This also allows to replace the following `#if HAVE_IFADDRS` with `#ifdef HAVE_IFADDRS`.
💬 hebasto commented on pull request "test: Suppress Windows abort message in CI":
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2867088113)
As an alternative, we could explicitly set our own `SUPPRESS_ABORT_MESSAGE_BOX` environment variable in the workflow, and gate the code with this variable instead, removing the `#if defined(_MSC_VER)` condition.
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2867088113)
As an alternative, we could explicitly set our own `SUPPRESS_ABORT_MESSAGE_BOX` environment variable in the workflow, and gate the code with this variable instead, removing the `#if defined(_MSC_VER)` condition.
💬 luke-jr commented on pull request "fs: use `ftruncate` in `AllocateFileRange` on OpenBSD":
(https://github.com/bitcoin/bitcoin/pull/32645#discussion_r2132921035)
We're already calling fallocate here... seems like this whole #if mess should be sorted out.
(https://github.com/bitcoin/bitcoin/pull/32645#discussion_r2132921035)
We're already calling fallocate here... seems like this whole #if mess should be sorted out.
💬 ajtowns commented on pull request "Bump SCRIPT_VERIFY flags to 64 bit":
(https://github.com/bitcoin/bitcoin/pull/32998#issuecomment-3082011233)
CI failure seems to be due to [a bug in qt6 6.4](https://bugreports.qt.io/browse/QTBUG-31496?focusedId=888930&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-888930):
> Also for people coming here from Google: I had the same error message, but for me the problem was that I was using Qt 6.4.2 which apparently thinks that #if 202002L < 201103L is true, which causes c++config.h to not be included (and no #error is generated because moc doesn't support the directiv
...
(https://github.com/bitcoin/bitcoin/pull/32998#issuecomment-3082011233)
CI failure seems to be due to [a bug in qt6 6.4](https://bugreports.qt.io/browse/QTBUG-31496?focusedId=888930&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-888930):
> Also for people coming here from Google: I had the same error message, but for me the problem was that I was using Qt 6.4.2 which apparently thinks that #if 202002L < 201103L is true, which causes c++config.h to not be included (and no #error is generated because moc doesn't support the directiv
...
💬 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")))
```
(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")))
```
💬 maflcko commented on issue "OpenBSD, NetBSD: `-reindex` is broken":
(https://github.com/bitcoin/bitcoin/issues/33128#issuecomment-3206135519)
Diff to reproduce on Linux:
```diff
diff --git a/src/util/fs_helpers.cpp b/src/util/fs_helpers.cpp
index be7f1ee5a2..ada3280dbc 100644
--- a/src/util/fs_helpers.cpp
+++ b/src/util/fs_helpers.cpp
@@ -200,7 +200,7 @@ void AllocateFileRange(FILE* file, unsigned int offset, unsigned int length)
}
ftruncate(fileno(file), static_cast<off_t>(offset) + length);
#else
-#if defined(HAVE_POSIX_FALLOCATE)
+#if 0
// Version using posix_fallocate
off_t nEndPos = (off_t)offset + length;
...
(https://github.com/bitcoin/bitcoin/issues/33128#issuecomment-3206135519)
Diff to reproduce on Linux:
```diff
diff --git a/src/util/fs_helpers.cpp b/src/util/fs_helpers.cpp
index be7f1ee5a2..ada3280dbc 100644
--- a/src/util/fs_helpers.cpp
+++ b/src/util/fs_helpers.cpp
@@ -200,7 +200,7 @@ void AllocateFileRange(FILE* file, unsigned int offset, unsigned int length)
}
ftruncate(fileno(file), static_cast<off_t>(offset) + length);
#else
-#if defined(HAVE_POSIX_FALLOCATE)
+#if 0
// Version using posix_fallocate
off_t nEndPos = (off_t)offset + length;
...
💬 stickies-v commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2336501679)
[C23 compiler support](https://en.cppreference.com/w/c/compiler_support/23.html) is still early and limited, but [typed enums](https://open-std.org/JTC1/SC22/WG14/www/docs/n3030.htm) would offer increased type safety to users that want it?
```C
/* C23-aware enum definitions */
#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L && !defined(__cplusplus)
/**
* For C23 and later, use a strongly-typed enum. This creates a distinct type
* which is not implicitly convert
...
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2336501679)
[C23 compiler support](https://en.cppreference.com/w/c/compiler_support/23.html) is still early and limited, but [typed enums](https://open-std.org/JTC1/SC22/WG14/www/docs/n3030.htm) would offer increased type safety to users that want it?
```C
/* C23-aware enum definitions */
#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L && !defined(__cplusplus)
/**
* For C23 and later, use a strongly-typed enum. This creates a distinct type
* which is not implicitly convert
...
📝 fanquake opened a pull request: "depends: systemtap 5.3"
(https://github.com/bitcoin/bitcoin/pull/33373)
The diff in the copied header is:
```diff
< #if __STDC_VERSION__ >= 199901L
---
> #if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
```
From
https://sourceware.org/git/?p=systemtap.git;a=commit;h=b8345d8e07b725a943a97b19aa4866e74baadd98.
(https://github.com/bitcoin/bitcoin/pull/33373)
The diff in the copied header is:
```diff
< #if __STDC_VERSION__ >= 199901L
---
> #if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
```
From
https://sourceware.org/git/?p=systemtap.git;a=commit;h=b8345d8e07b725a943a97b19aa4866e74baadd98.
💬 hodlinator commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2387108889)
Could do potentially do something like...
```C++
#if defined(WAIT_FOR_DEBUGGER) && defined(NDEBUG)
#error "Should only enable WAIT_FOR_DEBUGGER in debug builds"
#endif
```
...though there could be scenarios where one wants to debug release builds as well.
*/contrib/guix/symbol-check.py* could also potentially analyze the binary after the fact. LLM says that the *lief* module can find calls to `prctl` when dynamically linking and otherwise one could add a dependency on the *capstone* mod
...
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2387108889)
Could do potentially do something like...
```C++
#if defined(WAIT_FOR_DEBUGGER) && defined(NDEBUG)
#error "Should only enable WAIT_FOR_DEBUGGER in debug builds"
#endif
```
...though there could be scenarios where one wants to debug release builds as well.
*/contrib/guix/symbol-check.py* could also potentially analyze the binary after the fact. LLM says that the *lief* module can find calls to `prctl` when dynamically linking and otherwise one could add a dependency on the *capstone* mod
...