Skip to content
This repository has been archived by the owner on Oct 5, 2021. It is now read-only.

Add IsLocalAddr helper #55

Closed
wants to merge 1 commit into from
Closed

Add IsLocalAddr helper #55

wants to merge 1 commit into from

Conversation

bigs
Copy link
Contributor

@bigs bigs commented Jun 13, 2019

Necessary for libp2p/go-libp2p#657

@bigs bigs requested review from Stebalien and raulk June 13, 2019 21:10
@bigs bigs closed this Jun 13, 2019
@bigs bigs deleted the featu/is-loopback branch June 13, 2019 21:21
// Private4 and Private6 are well-known private networks
var Private4, Private6 []*net.IPNet
var privateCIDR4 = []string{
// Private4, Private6, Local4, and Local6 are well-known private networks
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest breaking this down into two declarations and contextualising godoc.

// localhost
"127.0.0.0/8",
// link local
"169.254.0.0/16",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it’s ok moving link locals here. I struggle to find a use case where these would be useful, but maybe I’m missing something, @Stebalien?

@@ -47,6 +55,10 @@ var unroutableCIDR6 = []string{
}

func init() {
privateCIDR4 = append(privateCIDR4, localCIDR4...)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is confusing. Could you move the append to the declaration? Also, does it make sense to keep locals here?

switch c.Protocol().Code {
case ma.P_IP6ZONE:
return true
default:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this down?

return true
default:
return false
case ma.P_IP4:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IPv4 is the hot path, so let’s make it the first one, then IPv6.

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.

2 participants