This is a backport of libsecp256k1's PR523
Details
Details
- Reviewers
Fabien - Group Reviewers
Restricted Project - Commits
- rSTAGING6d8c162de5fb: [secp256k1] scratch: add stack frame support
rABC6d8c162de5fb: [secp256k1] scratch: add stack frame support
ninja check-secp256k1
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- secpstackframe
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 8956 Build 15885: Default Diff Build & Tests Build 15884: arc lint + arc unit
Event Timeline
| src/secp256k1/src/scratch_impl.h | ||
|---|---|---|
| 48 ↗ | (On Diff #15425) | Should be <= since scratch->frame is a counter and not an index ? |
| src/secp256k1/src/tests.c | ||
| 275 ↗ | (On Diff #15425) | secp256k1_scratch_allocate_frame doesn't take a boolean argument, this looks like a typo for CHECK(secp256k1_scratch_allocate_frame(scratch, 500, 1) == 1); |
| src/secp256k1/src/scratch_impl.h | ||
|---|---|---|
| 48 ↗ | (On Diff #15425) | This would be inconsistent with, for example, then assertion in secp256k1_scratch_destroy above: VERIFY_CHECK(scratch->frame == 0);. If the frames are counted from 0, then there might still be an allocated frame when the scratch is destroyed, causing a memory leak. |
| src/secp256k1/src/scratch_impl.h | ||
|---|---|---|
| 48 ↗ | (On Diff #15425) | You are correct. This is counting, but the code is correct anyways, because if we are going to allocate a new frame, then we want the frame count to be bellow the max, that that at the end of the process we reach at most the max number of frames. |