💬 vasild commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1268148641)
We have already checked if UNIX sockets are supported.
```cpp
#if HAVE_SOCKADDR_UN
argsman.AddArg("-onion=<ip:port|path>", "Use separate SOCKS5 proxy to reach peers via Tor onion services, set -noonion to disable (default: -proxy). May be a local file path prefixed with 'unix:' if UNIX domain sockets are supported by the platform.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
#else
argsman.AddArg("-onion=<ip:port>", "Use separate SOCKS5 proxy to reach peers via Tor onion
...
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1268148641)
We have already checked if UNIX sockets are supported.
```cpp
#if HAVE_SOCKADDR_UN
argsman.AddArg("-onion=<ip:port|path>", "Use separate SOCKS5 proxy to reach peers via Tor onion services, set -noonion to disable (default: -proxy). May be a local file path prefixed with 'unix:' if UNIX domain sockets are supported by the platform.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
#else
argsman.AddArg("-onion=<ip:port>", "Use separate SOCKS5 proxy to reach peers via Tor onion
...
💬 MarcoFalke commented on pull request "guix: use GCC 12.3.0 to build releases":
(https://github.com/bitcoin/bitcoin/pull/27897#issuecomment-1699018185)
Can you try with:
```diff
diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp
index 740f461548..260f7f6645 100644
--- a/src/test/system_tests.cpp
+++ b/src/test/system_tests.cpp
@@ -49,11 +49,7 @@ BOOST_AUTO_TEST_CASE(run_command)
}
{
// An invalid command is handled by Boost
-#ifdef WIN32
- const int expected_error{wine_runtime ? 6 : 2};
-#else
const int expected_error{2};
-#endif
BOOST_CHECK_EXCEPTION(RunCommandParseJS
...
(https://github.com/bitcoin/bitcoin/pull/27897#issuecomment-1699018185)
Can you try with:
```diff
diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp
index 740f461548..260f7f6645 100644
--- a/src/test/system_tests.cpp
+++ b/src/test/system_tests.cpp
@@ -49,11 +49,7 @@ BOOST_AUTO_TEST_CASE(run_command)
}
{
// An invalid command is handled by Boost
-#ifdef WIN32
- const int expected_error{wine_runtime ? 6 : 2};
-#else
const int expected_error{2};
-#endif
BOOST_CHECK_EXCEPTION(RunCommandParseJS
...
💬 ajtowns commented on pull request "net: Use serialization parameters for CAddress serialization":
(https://github.com/bitcoin/bitcoin/pull/25284#discussion_r1315927911)
I think you could do:
```c++
#ifdef HAVE_CXX20
template<typename Stream>
concept ParamsStreamConcept = requires(Stream& s) { s.GetParams(); };
template<typename Stream>
concept PlainStreamConcept = !ParamsStreamConcept<Stream>;
#else
template<typename Stream>
bool ParamsStreamConcept = true;
template<typename Stream>
bool PlainStreamConcept = true;
#endif
/** Wrapper that overrides the GetParams() function of a stream (and hides GetVersion/GetType). */
template <typename Para
...
(https://github.com/bitcoin/bitcoin/pull/25284#discussion_r1315927911)
I think you could do:
```c++
#ifdef HAVE_CXX20
template<typename Stream>
concept ParamsStreamConcept = requires(Stream& s) { s.GetParams(); };
template<typename Stream>
concept PlainStreamConcept = !ParamsStreamConcept<Stream>;
#else
template<typename Stream>
bool ParamsStreamConcept = true;
template<typename Stream>
bool PlainStreamConcept = true;
#endif
/** Wrapper that overrides the GetParams() function of a stream (and hides GetVersion/GetType). */
template <typename Para
...
💬 hebasto commented on pull request "build: Patch Qt to handle minimum macOS version properly":
(https://github.com/bitcoin/bitcoin/pull/28775#issuecomment-1790792236)
> Can you explain the patch? What is the actual problem, and how does this fix it? Seems a bit odd to have to patch version detection checks inside Qt, can the correct values not be passed in from the outside?
In macOS SDK, the problematic `os/activity.h` header relies on the `OS_LOG_TARGET_HAS_10_12_FEATURES` macro:
```c++
#if OS_LOG_TARGET_HAS_10_12_FEATURES
#ifndef OS_ACTIVITY_OBJECT_API
#define OS_ACTIVITY_OBJECT_API 1
#endif // OS_ACTIVITY_OBJECT_API
#else
#if OS_ACTIVITY_OBJECT_A
...
(https://github.com/bitcoin/bitcoin/pull/28775#issuecomment-1790792236)
> Can you explain the patch? What is the actual problem, and how does this fix it? Seems a bit odd to have to patch version detection checks inside Qt, can the correct values not be passed in from the outside?
In macOS SDK, the problematic `os/activity.h` header relies on the `OS_LOG_TARGET_HAS_10_12_FEATURES` macro:
```c++
#if OS_LOG_TARGET_HAS_10_12_FEATURES
#ifndef OS_ACTIVITY_OBJECT_API
#define OS_ACTIVITY_OBJECT_API 1
#endif // OS_ACTIVITY_OBJECT_API
#else
#if OS_ACTIVITY_OBJECT_A
...
💬 fanquake commented on issue "Building a wallet with legacy support fails on OpenBSD 7.4":
(https://github.com/bitcoin/bitcoin/issues/28963#issuecomment-1834640327)
> In file included from ../dist/./../cxx/cxx_db.cpp:13:
> ./db_cxx.h:59:10: fatal error: 'iostream.h' file not found
The issue is some code like this in bdb:
```cpp
#ifdef HAVE_CXX_STDHEADERS
#include <iostream>
#include <exception>
#define __DB_STD(x) std::x
#else
#include <iostream.h>
#include <exception.h>
#define __DB_STD(x) x
#endif
```
For some reason, configure fails to detect C++ header availability, and `HAVE_CXX_STDHEADERS` is not defined. Is `g++` and a C++ standard
...
(https://github.com/bitcoin/bitcoin/issues/28963#issuecomment-1834640327)
> In file included from ../dist/./../cxx/cxx_db.cpp:13:
> ./db_cxx.h:59:10: fatal error: 'iostream.h' file not found
The issue is some code like this in bdb:
```cpp
#ifdef HAVE_CXX_STDHEADERS
#include <iostream>
#include <exception>
#define __DB_STD(x) std::x
#else
#include <iostream.h>
#include <exception.h>
#define __DB_STD(x) x
#endif
```
For some reason, configure fails to detect C++ header availability, and `HAVE_CXX_STDHEADERS` is not defined. Is `g++` and a C++ standard
...
⚠️ DMUNEY45 opened an issue: "> In file included from ../dist/./../cxx/cxx_db.cpp:13:"
(https://github.com/bitcoin/bitcoin/issues/28982)
> In file included from ../dist/./../cxx/cxx_db.cpp:13:
> ./db_cxx.h:59:10: fatal error: 'iostream.h' file not found
The issue is some code like this in bdb:
```cpp
#ifdef HAVE_CXX_STDHEADERS
#include <iostream>
#include <exception>
#define __DB_STD(x) std::x
#else
#include <iostream.h>
#include <exception.h>
#define __DB_STD(x) x
#endif
```
For some reason, configure fails to detect C++ header availability, and `HAVE_CXX_STDHEADERS` is not defined. Is `g++` and a
...
(https://github.com/bitcoin/bitcoin/issues/28982)
> In file included from ../dist/./../cxx/cxx_db.cpp:13:
> ./db_cxx.h:59:10: fatal error: 'iostream.h' file not found
The issue is some code like this in bdb:
```cpp
#ifdef HAVE_CXX_STDHEADERS
#include <iostream>
#include <exception>
#define __DB_STD(x) std::x
#else
#include <iostream.h>
#include <exception.h>
#define __DB_STD(x) x
#endif
```
For some reason, configure fails to detect C++ header availability, and `HAVE_CXX_STDHEADERS` is not defined. Is `g++` and a
...
💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1609954791)
bebfafcf98d8ed1432407b7603991d54f7cc26c2: it would be useful to have a quick recap here of which strategy is used by `QueryDefaultGateway` for which OS.
I think it will be (slightly) more readable to have a single `QueryDefaultGateway` implementation which then calls `QueryDefaultGatewayWindows`, `QueryDefaultGatewayMac` and `QueryDefaultGatewayLinuxBSD`.
It can start with:
```cpp
Assume(network == NET_IPV4 || network == NET_IPV6)
```
and end with
```cpp
#else return std::nullo
...
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1609954791)
bebfafcf98d8ed1432407b7603991d54f7cc26c2: it would be useful to have a quick recap here of which strategy is used by `QueryDefaultGateway` for which OS.
I think it will be (slightly) more readable to have a single `QueryDefaultGateway` implementation which then calls `QueryDefaultGatewayWindows`, `QueryDefaultGatewayMac` and `QueryDefaultGatewayLinuxBSD`.
It can start with:
```cpp
Assume(network == NET_IPV4 || network == NET_IPV6)
```
and end with
```cpp
#else return std::nullo
...
💬 hodlinator commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1651128863)
Would have written it closer to something like:
```suggestion
fs::path absolutePath;
#ifdef WIN32
if (const char* homedrive = std::getenv("HOMEDRIVE"), * homepath = std::getenv("HOMEPATH");
homedrive != nullptr && homedrive[0] != 0 && homepath != nullptr && homepath[0] != 0)
absolutePath = fs::path(homedrive) / fs::path(homepath);
else
absolutePath = fs::path("C:\\Users\\Satoshi");
#else
if (const char* home = std::getenv("HOME"); home != nullptr
...
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1651128863)
Would have written it closer to something like:
```suggestion
fs::path absolutePath;
#ifdef WIN32
if (const char* homedrive = std::getenv("HOMEDRIVE"), * homepath = std::getenv("HOMEPATH");
homedrive != nullptr && homedrive[0] != 0 && homepath != nullptr && homepath[0] != 0)
absolutePath = fs::path(homedrive) / fs::path(homepath);
else
absolutePath = fs::path("C:\\Users\\Satoshi");
#else
if (const char* home = std::getenv("HOME"); home != nullptr
...
⚠️ brunoerg opened an issue: "docs: "Fuzzing the Bitcoin Core P2P layer using Honggfuzz NetDriver" is outdated"
(https://github.com/bitcoin/bitcoin/issues/30957)
Similar to #28019. The following instruction is outdated and doesn't work:
```diff
$ git apply << "EOF"
diff --git a/src/compat/compat.h b/src/compat/compat.h
index 8195bceaec..cce2b31ff0 100644
--- a/src/compat/compat.h
+++ b/src/compat/compat.h
@@ -90,8 +90,12 @@ typedef char* sockopt_arg_type;
// building with a binutils < 2.36 is subject to this ld bug.
#define MAIN_FUNCTION __declspec(dllexport) int main(int argc, char* argv[])
#else
+#ifdef HFND_FUZZING_ENTRY_FUNCTION_CXX
+#
...
(https://github.com/bitcoin/bitcoin/issues/30957)
Similar to #28019. The following instruction is outdated and doesn't work:
```diff
$ git apply << "EOF"
diff --git a/src/compat/compat.h b/src/compat/compat.h
index 8195bceaec..cce2b31ff0 100644
--- a/src/compat/compat.h
+++ b/src/compat/compat.h
@@ -90,8 +90,12 @@ typedef char* sockopt_arg_type;
// building with a binutils < 2.36 is subject to this ld bug.
#define MAIN_FUNCTION __declspec(dllexport) int main(int argc, char* argv[])
#else
+#ifdef HFND_FUZZING_ENTRY_FUNCTION_CXX
+#
...
💬 maflcko commented on issue "docs: "Fuzzing the Bitcoin Core P2P layer using Honggfuzz NetDriver" is outdated":
(https://github.com/bitcoin/bitcoin/issues/30957#issuecomment-2376413368)
Should be trivial to rebase it, no?
```diff
diff --git a/src/compat/compat.h b/src/compat/compat.h
index 366c648ae7..1f3bf2b018 100644
--- a/src/compat/compat.h
+++ b/src/compat/compat.h
@@ -92,8 +92,12 @@ typedef char* sockopt_arg_type;
// building with a binutils < 2.36 is subject to this ld bug.
#define MAIN_FUNCTION __declspec(dllexport) int main(int argc, char* argv[])
#else
+#ifdef HFND_FUZZING_ENTRY_FUNCTION_CXX
+#define MAIN_FUNCTION HFND_FUZZING_ENTRY_FUNCTION_CXX(int
...
(https://github.com/bitcoin/bitcoin/issues/30957#issuecomment-2376413368)
Should be trivial to rebase it, no?
```diff
diff --git a/src/compat/compat.h b/src/compat/compat.h
index 366c648ae7..1f3bf2b018 100644
--- a/src/compat/compat.h
+++ b/src/compat/compat.h
@@ -92,8 +92,12 @@ typedef char* sockopt_arg_type;
// building with a binutils < 2.36 is subject to this ld bug.
#define MAIN_FUNCTION __declspec(dllexport) int main(int argc, char* argv[])
#else
+#ifdef HFND_FUZZING_ENTRY_FUNCTION_CXX
+#define MAIN_FUNCTION HFND_FUZZING_ENTRY_FUNCTION_CXX(int
...