net: drop GetIPAddress() and add GetStringIPAddresses()

[amery]

pretending there is a single "best" IP address is misleading

[karasz]

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
}

[amery]

@karasz very simple reason, ignorance.

[karasz]

On second thoughts I would even drop

else{
	continue
}

looks like consuming cycles for no gain.

[amery]

how about

for _, addr := range addrs {

  if addr.Is4In6() {
    addr = addr.Unmap()
  } else {
    addr = addr.WithZone("")
  }

  out = append(out, addr.String())
}

(branch amended accordingly)

[karasz]

What if the addr is invalid? I think you will get that if the interface has no IP associated.

[amery]

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?

[karasz]

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 out. It could be much simplified if we would not insist in naming interfaces to check.

[amery]

added a commit using netip.ParseAddr() instead of type casting on GetIPAddresses() @karasz

[amery]

Another option for GetIPAddresses() would be:

@karasz I've submitted a PR similar to this proposal to darvaza's implementation. darvaza-proxy/darvaza#41

[karasz]

LGTM