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
  1. purpleKarrot commented at 7:06 pm on November 23, 2024: none
    Fixes #1637
  2. Make static context const bfc3ae0735
  3. 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.

  4. real-or-random added the label bug on Nov 24, 2024
  5. real-or-random removed the label bug on Nov 24, 2024
  6. real-or-random added the label refactor/smell on Nov 24, 2024
  7. 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.

  8. 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.

  9. real-or-random approved
  10. 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.

  11. real-or-random added the label needs-changelog on Nov 25, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-03 21:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me