bugfix: don’t blow up on “ARG_CHECK(ctx != NULL)” #301

pull theuni wants to merge 1 commits into bitcoin-core:master from theuni:arg_check changing 3 files +17 −17
  1. theuni commented at 6:21 pm on September 1, 2015: contributor
    The obvious fix for #300. I’m not sure if this should be handled in the same macro though.
  2. apoelstra commented at 6:36 pm on September 1, 2015: contributor

    I think a better fix is to replace the affected ARG_CHECKs with VERIFY_CHECK. Or even to put a VERIFY_CHECK for ctx != NULL in the ARG_CHECK macro itself.

    I think we’ll get burned if we don’t terminate the program on null context, since it’s used inside of so many macros.

  3. sipa commented at 7:09 pm on September 1, 2015: contributor

    Agree with @apoelstra. This will just make the accidental passing of ctx==NULL harder to discover. On Sep 1, 2015 20:36, “Andrew Poelstra” notifications@github.com wrote:

    I think a better fix is to replace the affected ARG_CHECKs with VERIFY_CHECK. Or even to put a VERIFY_CHECK for ctx != NULL in the ARG_CHECK macro itself.

    I think we’ll get burned if we don’t terminate the program on null context, since it’s used inside of so many macros.

    — Reply to this email directly or view it on GitHub https://github.com/bitcoin/secp256k1/pull/301#issuecomment-136822520.

  4. theuni commented at 7:11 pm on September 1, 2015: contributor
    Ok, fine by me. This was just the “fix it as it lies” change. I’d much prefer to handle the ctx logic separately from the other args.
  5. apoelstra commented at 5:51 pm on September 2, 2015: contributor
    Can you replace this with a patch that replaces any ARG_CHECKs on ctx with VERIFY_CHECK? (If not I can do it.)
  6. theuni force-pushed on Sep 3, 2015
  7. theuni force-pushed on Sep 3, 2015
  8. theuni commented at 2:08 am on September 3, 2015: contributor
    Updated as suggested. Also moved one case where the context was checked after other arguments.
  9. theuni force-pushed on Sep 3, 2015
  10. theuni commented at 3:40 am on September 3, 2015: contributor
    Updated again. I accidentally left the original change to the ARG_CHECK macro in, gone now.
  11. bugfix: "ARG_CHECK(ctx != NULL)" makes no sense
    Move all context checks to VERIFY_CHECK and be sure they come before all
    ARG_CHECKs.
    b183b41122
  12. sipa merged this on Sep 4, 2015
  13. sipa closed this on Sep 4, 2015

  14. sipa referenced this in commit c822693eff on Sep 4, 2015
  15. sipa cross-referenced this on Sep 29, 2015 from issue Bad callback logic by theuni

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-10-30 09:15 UTC

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