💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071702348)
Changed to `LogError()`, but `__func__` is redundant with the config option `-logsourcelocations`, so I omitted it.

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071702885)
Right. Removed the scope.

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071703580)
Moved the comment before the actual closing. Reduced as well.

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071703786)
Done.

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071706458)
Removed this outdated comment.

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2847360895)
`c9cd7d3aa9...66087035cf`: remove outdated comment

👍 hebasto approved a pull request: "Shuffle depends instructions and recommend modern make for macOS"
(https://github.com/bitcoin/bitcoin/pull/32086#pullrequestreview-2812173999)
ACK ace66e125c01448a4bde0889b044544937bad6ea.

💬 hebasto commented on pull request "Shuffle depends instructions and recommend modern make for macOS":
(https://github.com/bitcoin/bitcoin/pull/32086#discussion_r2071707186)
`gmake` is used in `doc/build-freebsd.md`.

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071707907)
I think it is better to stop the test right away if the closing fails.

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071708123)
Done.

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071708360)
Done.

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071715279)
Hmm, the OS `feof()` returns nonzero if end-of-file. This makes it possible to use the natural:

```cpp
if (feof()) { // end-of-file has been reached
```

But `fclose()` returns zero if the file has been closed. I do not want to invert the return value because that would be confusing for people that know the interface of the OS `fclose()`. Or even more confusing, if the return is not inverted and converted to `bool`:

```
bool fclose(...);

if (fclose()) { // error!
```

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071716526)
Elaborated a little bit:

```cpp
// Callers that wrote to the file must have closed it explicitly
// with the fclose() method and checked that the close succeeded.
// This is because here from the destructor we have no way to signal
// error due to close which, after write, could mean the file is
// corrupted and must be handled properly at the call site.
// Destructors in C++ cannot signal an error to the callers be
...

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071717206)
I think it is better to stop the test immediately if closing a file gives an error.

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071717840)
What to be added to `AutoFile::write_buffer`?

💬 hebasto commented on issue "build: x86 afl++ ASan build broken "error: inline assembly requires more registers than available"":
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2847384462)
My guess is that `/AFLplusplus/afl-clang-fast++` fails to handle the `-fcf-protection=full -fcf-protection=none` flags properly.

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2847391766)
`66087035cf...fe2d1955f6`: add missing includes in the first commit

✅ hebasto closed a pull request: "[POC] cmake: Introduce LLVM's Source-based Code Coverage reports"
(https://github.com/bitcoin/bitcoin/pull/31394)

💬 mzumsande commented on pull request "cli: add -usefile option":
(https://github.com/bitcoin/bitcoin/pull/32399#issuecomment-2847462346)
> I've been using `-stdin` (e.g. `cat block.hex | bitcoin-cli -stdin submitblock`) for this. Would this work here too?

Oh, I didn't think of that - I tried it out and I think that would work too - that would make it simpler, no changes to bitcoin-cli necessary, will change to that approach.

💬 brunoerg commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r2071773924)
Yes, these other things are also not necessary when using in memory db. I'm gonna address it.

💬 mzumsande commented on pull request "cli: add -usefile option":
(https://github.com/bitcoin/bitcoin/pull/32399#issuecomment-2847489011)
Since this will only change the test framework now, I'll just add it to #32290 - no extra PR needed anymore.