Skip to content
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

net: Correct comparison of addr count #15345

Merged
merged 1 commit into from Feb 4, 2019

Conversation

Projects
None yet
5 participants
@dongcarl
Copy link
Contributor

commented Feb 4, 2019

LOCAL_NONE is supposed to be an enum indicating the nScore of a
LocalServiceInfo rather than the count of an addr in mapLocalHost.

net: Correct comparison of addr count
LOCAL_NONE is supposed to be an enum indicating the score of a
LocalServiceInfo rather than the count of an addr in mapLocalHost.
@@ -174,8 +174,7 @@ CAddress GetLocalAddress(const CNetAddr *paddrPeer, ServiceFlags nLocalServices)
static int GetnScore(const CService& addr)
{
LOCK(cs_mapLocalHost);
if (mapLocalHost.count(addr) == LOCAL_NONE)

This comment has been minimized.

Copy link
@laanwj

laanwj Feb 4, 2019

Member

Huh, this is really strange. I guess it's correct because LOCAL_NONE effectively is 0 but it's certainly not right.

@laanwj

This comment has been minimized.

Copy link
Member

commented Feb 4, 2019

utACK 107623c

@laanwj laanwj added the P2P label Feb 4, 2019

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Feb 4, 2019

ACK 107623c (also checked that this commit doesn't change the binary produced by clang7 on my system, so tagged with "refactoring")

@MarcoFalke MarcoFalke added this to the 0.18.0 milestone Feb 4, 2019

@promag

This comment has been minimized.

Copy link
Member

commented Feb 4, 2019

utACK 107623c, although it could improve a bit by avoiding 2nd lookup.

std::map<CNetAddr, LocalServiceInfo>::const_iterator it = mapLocalHost.find(addr);
return it == mapLocalHost.end() ? 0 : it->second.nScore;
@jonasschnelli

This comment has been minimized.

Copy link
Member

commented Feb 4, 2019

utACK 107623c

@laanwj

This comment has been minimized.

Copy link
Member

commented Feb 4, 2019

utACK 107623c, although it could improve a bit by avoiding 2nd lookup.

I thought about commenting that, but I don't think that's really much of an improvement in readability (and this isn't exactly a critical path for perfomance).

@laanwj laanwj merged commit 107623c into bitcoin:master Feb 4, 2019

1 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Feb 4, 2019

Merge #15345: net: Correct comparison of addr count
107623c net: Correct comparison of addr count (Carl Dong)

Pull request description:

  `LOCAL_NONE` is supposed to be an enum indicating the `nScore` of a
  `LocalServiceInfo` rather than the count of an addr in `mapLocalHost`.

Tree-SHA512: a47a0859dd11c991d75b54e96b08c502e3d235f7a6522a2355053f377d05e7853483996919292f458d917a561b23951e6945d6bf0ff5a2f29513c477c640bdd2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.