net: `interface_linux.go` has redundant code

[wlynxg]

What version of Go are you using (go version)?

$ go version
go version go1.20 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env

What did you do?

func addrTable(ift []Interface, ifi *Interface, msgs []syscall.NetlinkMessage) ([]Addr, error) {
	var ifat []Addr
loop:
	for _, m := range msgs {
		switch m.Header.Type {
		case syscall.NLMSG_DONE:
			break loop
		case syscall.RTM_NEWADDR:
			ifam := (*syscall.IfAddrmsg)(unsafe.Pointer(&m.Data[0]))
			if len(ift) != 0 || ifi.Index == int(ifam.Index) {
				if len(ift) != 0 {
					var err error
					ifi, err = interfaceByIndex(ift, int(ifam.Index))
					if err != nil {
						return nil, err
					}
				}
				attrs, err := syscall.ParseNetlinkRouteAttr(&m)
				if err != nil {
					return nil, os.NewSyscallError("parsenetlinkrouteattr", err)
				}
				ifa := newAddr(ifam, attrs)
				if ifa != nil {
					ifat = append(ifat, ifa)
				}
			}
		}
	}
	return ifat, nil
}

In the above code, the following code logic seems to be an invalid code logic.

if len(ift) != 0 {
    var err error
    ifi, err = interfaceByIndex(ift, int(ifam.Index))
    if err != nil {
        return nil, err
    }
}

len(ift) != 0 || ifi.Index == int(ifam.Index) never executes ifi.Index == int(ifam.Index) when len(ift) != 0,
therefore reassigning ifi has no effect.

What did you expect to see?

What did you see instead?

[panjf2000]

I assume what you meant is that it's needless to call interfaceByIndex reassigning the ifi when len(ift) != 0 and ifi.Index == int(ifam.Index) are both true, right?

[wlynxg]

I assume what you meant is that it's needless to call interfaceByIndex reassigning the ifi when len(ift) != 0 and ifi.Index == int(ifam.Index) are both true, right?

When len(ift) != 0 is true, the judgment logic of ifi.Index == int(ifam.Index) will not take effect, so there is no point in reassigning ifi.

[ianlancetaylor]

I agree that this code doesn't make sense, but what I don't know is what it is supposed to do. When this code was introduced in https://go.dev/cl/7400055, it made sense because ifi was passed to newAddr. That was changed in https://go.dev/cl/37892, because newAddr didn't use that parameter. But newAddr used to use the parameter; that part changed in (I think) https://go.dev/cl/11352. So I guess that we could just stop setting ifi in the loop today. But I'd rather that somebody who knows what this code is doing double-check that it is doing what it is supposed to do.

[aimuz]

Can I try to finish the job?

[wlynxg]

Can I try to finish the job?

Now we need to wait for the person who left this code to confirm that deleting this code will not have some other effects.

[aimuz]

Okey