💬 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
...
💬 hebasto commented on pull request "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256":
(https://github.com/bitcoin/bitcoin/pull/29180#discussion_r1442214009)
nit:
```suggestion
#endif // DISABLE_OPTIMIZED_SHA256
```
(https://github.com/bitcoin/bitcoin/pull/29180#discussion_r1442214009)
nit:
```suggestion
#endif // DISABLE_OPTIMIZED_SHA256
```
👍 ryanofsky approved a pull request: "refactor: Compile unreachable walletdb code"
(https://github.com/bitcoin/bitcoin/pull/29315#pullrequestreview-1844216896)
Code review ACK fa3373d3adbace7e4665cf391363319a55a09a96. This looks good, and should prevent code in the else blocks from accidentally breaking.
Maybe a way to make it a little clearer would be to define `use_bdb` and `use_sqlite` c++ constants like:
```
constexpr bool use_bdb{
#ifdef USE_BDB
true
#endif
};
```
Then the actual code could use `if constexpr(use_bsb)` like a normal conditional. This might be more verbose than the current approach, though, and the current approach do
...
(https://github.com/bitcoin/bitcoin/pull/29315#pullrequestreview-1844216896)
Code review ACK fa3373d3adbace7e4665cf391363319a55a09a96. This looks good, and should prevent code in the else blocks from accidentally breaking.
Maybe a way to make it a little clearer would be to define `use_bdb` and `use_sqlite` c++ constants like:
```
constexpr bool use_bdb{
#ifdef USE_BDB
true
#endif
};
```
Then the actual code could use `if constexpr(use_bsb)` like a normal conditional. This might be more verbose than the current approach, though, and the current approach do
...
💬 theuni commented on pull request "serialization: c++20 endian/byteswap/clz modernization":
(https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1922334399)
Aha, I finally tracked down the culprit! No wonder the benchmarks weren't making sense!
The problem was the removal of this from `crypto/common.h`:
```c++
#if defined(HAVE_CONFIG_H)
#include <config/bitcoin-config.h>
#endif
```
Turns out, `sha256.cpp` was relying on that. Adding that include to `sha256.cpp` (where it belongs) fixes my local benchmarks.
I'm going to put the clz changes back to match the PR title/description, but this time with the include fixed up. Hopefully after t
...
(https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1922334399)
Aha, I finally tracked down the culprit! No wonder the benchmarks weren't making sense!
The problem was the removal of this from `crypto/common.h`:
```c++
#if defined(HAVE_CONFIG_H)
#include <config/bitcoin-config.h>
#endif
```
Turns out, `sha256.cpp` was relying on that. Adding that include to `sha256.cpp` (where it belongs) fixes my local benchmarks.
I'm going to put the clz changes back to match the PR title/description, but this time with the include fixed up. Hopefully after t
...
💬 vasild commented on issue "Clang 14 emits `-Wunreachable-code` warnings":
(https://github.com/bitcoin/bitcoin/issues/29334#issuecomment-1948185285)
The code in question can be improved:
```cpp
#ifdef USE_BDB
if constexpr (true) {
return MakeBerkeleyDatabase(path, options, status, error);
} else
#endif
{
error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support Berkeley DB database format.", fs::PathToString(path)));
status = DatabaseStatus::FAILED_BAD_FORMAT;
return nullptr;
}
```
If `USE_BDB` is defined then this becomes:
```cpp
if c
...
(https://github.com/bitcoin/bitcoin/issues/29334#issuecomment-1948185285)
The code in question can be improved:
```cpp
#ifdef USE_BDB
if constexpr (true) {
return MakeBerkeleyDatabase(path, options, status, error);
} else
#endif
{
error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support Berkeley DB database format.", fs::PathToString(path)));
status = DatabaseStatus::FAILED_BAD_FORMAT;
return nullptr;
}
```
If `USE_BDB` is defined then this becomes:
```cpp
if c
...
💬 hebasto commented on pull request "Fix misleading signmessage error with segwit":
(https://github.com/bitcoin-core/gui/pull/819#discussion_r1588327277)
It is https://github.com/bitcoin/bitcoin/pull/29494 goal to include this headers unguarded :)
Effectively, this PR conflicts with it. However, I believe it will be merged soon.
Anyway, it seems reasonable in this PR to use:
```c++
#if defined(HAVE_CONFIG_H)
#include <config/bitcoin-config.h>
#endif
```
or
```c++
#include <config/bitcoin-config.h> // IWYU pragma: keep
```
(https://github.com/bitcoin-core/gui/pull/819#discussion_r1588327277)
It is https://github.com/bitcoin/bitcoin/pull/29494 goal to include this headers unguarded :)
Effectively, this PR conflicts with it. However, I believe it will be merged soon.
Anyway, it seems reasonable in this PR to use:
```c++
#if defined(HAVE_CONFIG_H)
#include <config/bitcoin-config.h>
#endif
```
or
```c++
#include <config/bitcoin-config.h> // IWYU pragma: keep
```
💬 vasild commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1618645388)
This is the upstream fix: https://github.com/freebsd/freebsd-src/pull/1070.
The suggested workaround to define `typeof` is ok:
```diff
diff --git i/src/util/netif.cpp w/src/util/netif.cpp
index 845b8aed1d..1840974a83 100644
--- i/src/util/netif.cpp
+++ w/src/util/netif.cpp
@@ -9,18 +9,24 @@
#include <logging.h>
#include <netbase.h>
#include <util/check.h>
#include <util/sock.h>
#include <util/syserror.h>
+#ifdef __FreeBSD__
+#include <osreldate.h>
+#endif
+
// Linux
...
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1618645388)
This is the upstream fix: https://github.com/freebsd/freebsd-src/pull/1070.
The suggested workaround to define `typeof` is ok:
```diff
diff --git i/src/util/netif.cpp w/src/util/netif.cpp
index 845b8aed1d..1840974a83 100644
--- i/src/util/netif.cpp
+++ w/src/util/netif.cpp
@@ -9,18 +9,24 @@
#include <logging.h>
#include <netbase.h>
#include <util/check.h>
#include <util/sock.h>
#include <util/syserror.h>
+#ifdef __FreeBSD__
+#include <osreldate.h>
+#endif
+
// Linux
...
💬 nicholas-1vbw commented on issue "Risczero Fit":
(https://github.com/bitcoin/bitcoin/issues/30747#issuecomment-2320560690)
We compiled a scratch main.cpp that includes `assumptions.h` successfully, but fails with `static_assert(std::is_same_v<int, int32_t>);`
Actually, the toolchain from risc0 has different type of "int" and "int32_t", see https://github.com/risc0/toolchain/releases/tag/2024.01.05
int32_t is defined in "riscv32-unknown-elf/include/sys/_stdint.h"
```
#ifdef ___int32_t_defined
#ifndef _INT32_T_DECLARED
typedef __int32_t int32_t ;
#define _INT32_T_DECLARED
#endif
#ifndef _UINT32_T_DECLARED
...
(https://github.com/bitcoin/bitcoin/issues/30747#issuecomment-2320560690)
We compiled a scratch main.cpp that includes `assumptions.h` successfully, but fails with `static_assert(std::is_same_v<int, int32_t>);`
Actually, the toolchain from risc0 has different type of "int" and "int32_t", see https://github.com/risc0/toolchain/releases/tag/2024.01.05
int32_t is defined in "riscv32-unknown-elf/include/sys/_stdint.h"
```
#ifdef ___int32_t_defined
#ifndef _INT32_T_DECLARED
typedef __int32_t int32_t ;
#define _INT32_T_DECLARED
#endif
#ifndef _UINT32_T_DECLARED
...
💬 dergoegge commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2349317870)
>Would it make sense to additionally have some belt-and-suspenders check somewhere at the start of bitcoind that FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION is not set, so it's not only in the build system?
I guess we could (e.g. in `bitcoind.cpp`) do something like:
```c++
#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
static_assert(false, "bitcoind can't be build with FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION");
#endif
```
But I would be surprised if our existing tests do not already
...
(https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2349317870)
>Would it make sense to additionally have some belt-and-suspenders check somewhere at the start of bitcoind that FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION is not set, so it's not only in the build system?
I guess we could (e.g. in `bitcoind.cpp`) do something like:
```c++
#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
static_assert(false, "bitcoind can't be build with FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION");
#endif
```
But I would be surprised if our existing tests do not already
...