Skip to content

Conversation

casey
Copy link
Contributor

@casey casey commented Aug 17, 2015

This PR consists of three commits.

The first fixes a bug where the size of a limited map was actually one less than the maximum size set. This shouldn't matter at all for the existing code, since there is only one limitedmap, and its size is very large.

The second disallows unlimited limited maps, since that doesn't make very much sense, and simplifies the code a little bit. (I'm not too attached to this, in case anyone doesn't like this change I can just remove it.)

The third adds unit tests for limitedmap, which were previously absent.

I elected to leave the commits unsquashed for review, since they're logically separate, but I can squash if desired.

@casey casey changed the title Limitedmap fixes limitedmap fixes and tests Aug 17, 2015
@sipa
Copy link
Member

sipa commented Aug 17, 2015

ACK

@TheBlueMatt
Copy link
Contributor

utACK

@laanwj
Copy link
Member

laanwj commented Aug 19, 2015

utACK. Thanks a lot for adding tests!

@laanwj laanwj merged commit 7bd57bb into bitcoin:master Aug 19, 2015
laanwj added a commit that referenced this pull request Aug 19, 2015
7bd57bb Add limitedmap test (Casey Rodarmor)
8b06894 Disallow unlimited limited maps (Casey Rodarmor)
fd2d862 Make limited map actually respect max size (Casey Rodarmor)
@casey casey deleted the limitedmap branch August 19, 2015 14:39
@dcousens
Copy link
Contributor

Is it worth adding a test to verify that insert after max_size is reached, it doesn't increment size?
(Happy to do this)

@casey
Copy link
Contributor Author

casey commented Aug 20, 2015

It does this already, unless you were thinking of a more substantial test: 11 elements are added (one one off, and then ten in a loop) and then line 39 checks that there are only 10 elements.

@dcousens
Copy link
Contributor

Sorry, I missed that first insert @casey. utACK :)

luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Jan 9, 2016
luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Jan 9, 2016
Github-Pull: bitcoin#6561
Rebased-From: 7bd57bb
luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Jan 10, 2016
zkbot added a commit to zcash/zcash that referenced this pull request Feb 19, 2021
Bitcoin 0.12 misc P2P/Net PRs

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#5288
  - Only the reorg, option was removed in bitcoin/bitcoin#6374 which we merged in #1258
- bitcoin/bitcoin#6561
- bitcoin/bitcoin#6728
- bitcoin/bitcoin#6829
- bitcoin/bitcoin#6974
- bitcoin/bitcoin#7075
- bitcoin/bitcoin#7166

Part of #2074.
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants