-
Notifications
You must be signed in to change notification settings - Fork 37.7k
limitedmap fixes and tests #6561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ACK |
utACK |
utACK. Thanks a lot for adding tests! |
Is it worth adding a test to verify that |
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. |
Sorry, I missed that first insert @casey. utACK :) |
Github-Pull: bitcoin#6561 Rebased-From: fd2d862
Github-Pull: bitcoin#6561 Rebased-From: 7bd57bb
Github-Pull: bitcoin#6561 Rebased-From: fd2d862
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.
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.