Fixes #1637
Make static context const #1639
pull purpleKarrot wants to merge 1 commits into bitcoin-core:master from purpleKarrot:static-context-constness changing 2 files +4 −4-
purpleKarrot commented at 7:06 PM on November 23, 2024: contributor
-
real-or-random commented at 9:45 PM on November 24, 2024: contributor
Looks good to me.
The only caveat is that this is, strictly speaking, a breaking change. If we want to be strict, we'd need a 0.7.0 instead of a 0.6.1. But I don't think we care?! I mean, any user code writing to the variable is most likely buggy... Hm.
- real-or-random added the label bug on Nov 24, 2024
- real-or-random removed the label bug on Nov 24, 2024
- real-or-random added the label refactor/smell on Nov 24, 2024
-
sipa commented at 9:50 PM on November 24, 2024: contributor
Technically speaking, this is certainly an API change. A constant and non-constant variable are not compatible as far as the C language is concerned. A (buggy) program could fail to compile with this change, while it was fine before.
But is it an ABI break? The symbol still exists, and has the same size and meaning. Linking will not fail for any reason, it is only at runtime that an attempt to modify the variable may (or may not) trigger a memory violation.
-
real-or-random commented at 10:12 PM on November 24, 2024: contributor
it is only at runtime that an attempt to modify the variable may (or may not) trigger a memory violation.
The good thing is that a clean crash is probably the worst that can happen in practice...
I think I'm okay with anything here. If you ask me, we can treat this as a breaking or a non-breaking chance, when it comes to both API and ABI.
- real-or-random approved
-
real-or-random commented at 5:39 PM on November 25, 2024: contributor
In any case, I prefer this proper fix over introducing a second symbol just to keep backwards compatibility.
utACK bfc3ae0735597b676a0472ab6cbd0f909ee30195
We can debate at release time whether this is breaking or not.
- real-or-random added the label needs-changelog on Nov 25, 2024
-
real-or-random commented at 3:08 PM on February 24, 2025: contributor
cc @jonasnick @sipa
Can you take a look? I think this will be good to have merged because it's addressing a real user issue.
-
sipa commented at 3:13 PM on February 24, 2025: contributor
ACK
-
Make static context const 432ac57705
-
purpleKarrot commented at 4:34 PM on February 24, 2025: contributor
I just rebased and signed the commit.
- real-or-random approved
-
real-or-random commented at 9:52 AM on February 25, 2025: contributor
ACK 432ac57705b8e2f6a5874095214c443f1f8ea9bf
- real-or-random merged this on Feb 25, 2025
- real-or-random closed this on Feb 25, 2025
- hebasto referenced this in commit 0ee5708432 on Mar 10, 2025
- hebasto referenced this in commit dd59896431 on Mar 17, 2025
- fjahr referenced this in commit cd1404164e on Mar 23, 2025
- purpleKarrot deleted the branch on Apr 6, 2025
- hebasto referenced this in commit 017ff1c67c on May 8, 2025
- hebasto referenced this in commit c31fcaaad3 on May 13, 2025
- fanquake referenced this in commit 8309a9747a on May 13, 2025
- vmta referenced this in commit 3a0314c68f on May 22, 2025
- real-or-random referenced this in commit 6037833c9e on Jul 21, 2025
- fanquake removed the label needs-changelog on Jul 29, 2025
-
- janus referenced this in commit 498ffcb50b on Sep 7, 2025
- real-or-random referenced this in commit 6e071d18ba on Feb 23, 2026
- github-actions[bot] referenced this in commit 758d4e90b4 on Mar 1, 2026
- github-actions[bot] referenced this in commit 68a2178f22 on Mar 1, 2026
- github-actions[bot] referenced this in commit a8bc1a0b2b on Mar 1, 2026
- 0x000000000019d6689c085ae165831e934ff76 referenced this in commit 3b9450150d on Mar 2, 2026
- csjones referenced this in commit a4d92824ae on Mar 2, 2026
