util: add missing include and fix function signature #27192
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
| @@ -4,14 +4,15 @@ | |||
| // file COPYING or http://www.opensource.org/licenses/mit-license.php. | |||
|
|
|||
| #include <fs.h> | |||
| #include <util/readwritefile.h> | |||
There was a problem hiding this comment.
nit: Including the own module goes on the first line/section, usually, to catch missing includes in the header file.
Also, could add it to iwyu in ci/test/06_...sh, so that reviewers can look at the ci output if they want?
There was a problem hiding this comment.
Reordered. I think this change is simple enough for manual review :)
There was a problem hiding this comment.
Yeah, I was mostly thinking that it would be good to check for other missing includes while touching the file, so that it doesn't have to be touched again later if there is a missing stdlib include or so.
101601a to
8847ce4
Compare
…ture 8847ce4 util: add missing include and fix function signature (Cory Fields) Pull request description: ping hebasto Discovered while testing pre-compiled header support with CMake: https://github.com/theuni/bitcoin/commits/cmake-pch-poc. Compilation of that branch fails without this fix and succeeds with it. Similar to the fix in bitcoin#27144. The problem of having a default argument in the definition was masked by the missing include. Using PCH forces that include, so we end up with the compiler error we should've been getting all along. ACKs for top commit: fanquake: ACK 8847ce4 Tree-SHA512: 5eb9a6691ee37cbc5033a48aedcbf5c93af3b234614ae14c3fcc858f1ee505f630ad68f8bbb69ffa280080c8d0f91451362cb3819290b741ce906b2b3224a622
|
Post-merge ACK. |
ping hebasto
Discovered while testing pre-compiled header support with CMake: https://github.com/theuni/bitcoin/commits/cmake-pch-poc. Compilation of that branch fails without this fix and succeeds with it.
Similar to the fix in #27144.
The problem of having a default argument in the definition was masked by the missing include. Using PCH forces that include, so we end up with the compiler error we should've been getting all along.