This is not a hardening specific flag, it should be used at all times, regardless of if hardening is enabled or not. Note that this was still the case here, but having this exist in the hardening flags is confusing, and may lead someone to move it inside one of the use_hardening blocks, where it would become unused, with --disable-hardening.
Noticed while reviewing https://github.com/hebasto/bitcoin/pull/32#discussion_r1363564161.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For detailed information about the code coverage, see the test coverage report.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | theuni, hebasto, luke-jr, TheCharlatan |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
This is not a hardening specific flag, it should be used at all times, regardless of if hardening is enabled or not. Note that this was still the case here, but having this exist in the hardening flags is confusing, and may lead someone to move it inside one of the
use_hardeningblocks, where it would become unused, with--disable-hardening.
I viewed it "confusing" in the same way while working on CMake-based build system.
Concept ACK.
Should we also consider any of other related bugs mentioned in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111843?
This is not a hardening specific flag, it should be used at all times,
regardless of if hardening is enabled or not. Note that this was
still the case here, but having this exist in the hardening flags is
confusing, and may lead someone to move it inside one of the `use_hardening`
blocks, where it would become unused, with `--disable-hardening`.
Should we also consider any of other related bugs mentioned in
I've switched to linking to the meta issue, over a specific bug report, but I'm not sure what else you're suggesting to do.
ACK 8cfa22a8464c07da3bb47e7db323ed1069b97457. Agree it's confusing as-is and this better matches the intent.
ACK 8cfa22a8464c07da3bb47e7db323ed1069b97457
<!--9cd9c72976c961c55c7acef8f6ba82cd-->
| File | commit 655dc716aa6043613171a1338e22928de89a7d3e<br>(master) | commit 4284cffb7114605136cd55787bc66371538d5d30<br>(master and this pull) |
|---|---|---|
| guix_build.log | cebd814ed065d782... |
3c834795531c6e99... |
| guix_build.log.diff | 475dcd577b86fb19... |
utACK 8cfa22a8464c07da3bb47e7db323ed1069b97457
ACK 8cfa22a8464c07da3bb47e7db323ed1069b97457