False assumption: IPs appear in a single position

[tomjn]

https://github.com/goodhosts/hostsfile/blob/master/hosts.go#L103

The code that adds a host performs a check to see if a line with the IP is already present, and if so, it inspects the hosts on that line and only adds the host if it is not listed.

However, this assumption is not true.

For example, consider this hosts file:

127.0.0.1 tom.test
127.0.0.1 tom.test example.test

Goodhosts will pick up the first line, see that example.test is not present, and add the host, leading to duplication.

Instead, this needs to be a loop, and position := h.getIpPosition(ip) needs to be positions := h.getIpPositions(ip).

Likewise, if an existing entry is present, this code does not remove it, which can lead to hostnames being defined multiple times but with different IPs

[tomjn]

Likewise getHostPosition getHostPosition and getHostnamePosition need plural versions

[luthermonson]

good find!

[tomjn]

I notice that in places in the code it uses this and then loops until no instances are found, and in other places it might not do this.

Instead, if it fetched all positions, sorted them from smallest to largest, then worked backwards from the end of the file to the start, we can achieve the same result using these functions. I did start on adding the functions which seemed easy enough, but didn't look into using them, golang isn't super familiar to me. Something like this is what I was thinking:

func (h Hosts) getIpPositions(ip string) []int {
	positions := new [...]int
	for i := range h.Lines {
		line := h.Lines[i]
		if !line.IsComment() && line.Raw != "" && line.IP == ip {
			append(positions, i)
		}
	}

	return positions
}