-
Notifications
You must be signed in to change notification settings - Fork 0
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: drop GetIPAddress() and add GetStringIPAddresses() #5
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this a bit better? I do not get why you want to pull it through net.IP just to make it String later.....
// GetStringIPAddresses returns IP addresses as string
func GetStringIPAddresses(ifaces ...string) ([]string, error) {
addrs, err := GetIPAddresses(ifaces...)
out := make([]string, 0, len(addrs))
for _, addr := range addrs {
if addr.IsValid(){
if addr.Is4() || addr.Is6 {
addr=addr.WithZone("")
out = append(out, addr.String())
}elseif addr.Is4In6() {
addr=addr.Unmap()
out = append(out, addr.String())
}
}else{
continue
}
return out, err
}
@karasz very simple reason, ignorance. |
On second thoughts I would even drop else{
continue
} looks like consuming cycles for no gain. |
how about for _, addr := range addrs {
if addr.Is4In6() {
addr = addr.Unmap()
} else {
addr = addr.WithZone("")
}
out = append(out, addr.String())
} (branch amended accordingly) |
What if the addr is invalid? I think you will get that if the interface has no IP associated. |
I'm assuming GetIPAddresses() will only return valid ones. No addresses should result in an empty array Naïve? |
Another option for GetIPAddresses() would be: // GetIPAddresses returns the list of IP Addresses,
// optionally considering only the given interfaces
func GetIPAddresses(ifaces ...string) ([]netip.Addr, error) {
var out []netip.Addr
if len(ifaces) == 0 {
var err error
netAddrs, err := net.InterfaceAddrs()
if err != nil {
return out, err
}
for _, netAddr := range netAddrs {
addr, err := netip.ParseAddr(netAddr.String())
if err != nil {
return out, err
}
if addr.IsValid() {
out = append(out, addr)
}
}
}
for _, name := range ifaces {
ifi, err := net.InterfaceByName(name)
if err != nil {
return out, err
}
netAddrs, err := ifi.Addrs()
if err != nil {
return out, err
}
for _, netAddr := range netAddrs {
addr, err := netip.ParseAddr(netAddr.String())
if err != nil {
return out, err
}
if addr.IsValid() {
out = append(out, addr)
}
}
}
return out, nil
} This version might be a bit of overkill but will always return valid |
… cast Signed-off-by: Alejandro Mery <[email protected]>
Signed-off-by: Alejandro Mery <[email protected]>
Signed-off-by: Alejandro Mery <[email protected]>
added a commit using |
@karasz I've submitted a PR similar to this proposal to darvaza's implementation. darvaza-proxy/darvaza#41 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
pretending there is a single "best" IP address is misleading