Skip to content

Conversation

theStack
Copy link
Contributor

This PR gets rid of another unnecessary use of raw pointers, similar to PR #19053 (see also issue #19062 where useful commands for finding potential candidates are listed) but in the tx verification module.

For the functions CalculateSequenceLocks() and SequenceLocks(), the prevHeights vector parameter type is changed to be passed as a reference. Note that there were no checks for null pointers -- if one would pass nullptr to one of the functions, the following line would immediately lead to a crash:

assert(prevHeights->size() == tx.vin.size());

affects "prevHeights" parameter of the functions
- CalculateSequenceLocks()
- SequenceLocks()
@practicalswift
Copy link
Contributor

practicalswift commented May 26, 2020

Concept ACK: having compiler enforcement of preconditions is better than not having it (obviously) :)

@Empact
Copy link
Contributor

Empact commented May 26, 2020

Code Review ACK b00266f

@DrahtBot
Copy link
Contributor

DrahtBot commented May 27, 2020

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@maflcko maflcko merged commit a79bca2 into bitcoin:master Jun 8, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 8, 2020
…tx_verify.{h,cpp}

b00266f refactor: replace pointers by references within tx_verify.{h,cpp} (Sebastian Falbesoner)

Pull request description:

  This PR gets rid of another unnecessary use of raw pointers, similar to PR bitcoin#19053 (see also issue bitcoin#19062 where useful commands for finding potential candidates are listed) but in the tx verification module.

  For the functions `CalculateSequenceLocks()` and `SequenceLocks()`, the  `prevHeights` vector parameter type is changed to be passed as a reference. Note that there were no checks for null pointers -- if one would pass `nullptr` to one of the functions, the following line would immediately lead to a crash:
  https://github.com/bitcoin/bitcoin/blob/dcacea096e029a02a937bf96d002ca7e94c48c15/src/consensus/tx_verify.cpp#L32

ACKs for top commit:
  Empact:
    Code Review ACK bitcoin@b00266f

Tree-SHA512: 0eb71591467905434082029128bdca4df94988c372af40dca325654f6c002c72a00c73776cb5e72d6de2b2f218649211a5dbf19300a2e01f1841d6034e0f01e0
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 20, 2020
Summary:
```
affects "prevHeights" parameter of the functions
- CalculateSequenceLocks()
- SequenceLocks()
```

Backport of core [[bitcoin/bitcoin#19069 | PR19069]].

Test Plan:
  ninja all check-all

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D8478
@theStack theStack deleted the 20200526-refactor-replace-pointers-by-refs-in-tx_verify branch December 1, 2020 09:55
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants