Repository navigation
fix(diam): return a nil listener or address on error - #73
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
When the SCTP listen failed, MultistreamListen returned
sctpListener{(*sctp.Listener)(nil)} with the error, and Listen converted
the typed nil *sctp.Listener into a non-nil net.Listener. A caller that
tested the listener before the error, or deferred its Close, took the
wrong branch or dereferenced nil. resolveAddress did the same with a
typed nil *sctp.Addr or *net.TCPAddr.
All three now return a nil interface with the error, as net.Listen and
net.ResolveTCPAddr do. Tests cover a bind error (EADDRNOTAVAIL), an
invalid port and a successful listen on every sctp and tcp network for
both listen functions, and resolveAddress's error and success results.
Fixes #68
6074e26 to
d6cc812
Compare
|
The lint failure is not from this change: staticcheck 2026.2.1 cannot read the export data of Go 1.27.2, the stable release CI now installs, so it stopped before analysing any package. Under Go 1.27.2, a staticcheck built from go-tools master (f1838cc3, which carries the fix) and golangci-lint both report no issues on this branch. The workflow pin is updated in a separate change. |
When the SCTP listen fails,
diam.MultistreamListenreturnedsctpListener{(*sctp.Listener)(nil)}together with the error, anddiam.Listenturned the typed nil*sctp.Listenerinto a non-nilnet.Listener. A caller that tests the listener before the error, or defersl.Close(), took the wrong branch or dereferenced nil. An audit of the dial and listen paths found the same pattern inresolveAddress, which could return a typed nil*sctp.Addror*net.TCPAddrwith its error.All three now return a nil interface with the error, as
net.Listendoes.Tests
TestListenResultcoversListenandMultistreamListenonsctp,sctp4,sctp6,tcp,tcp4andtcp6, with three cases each:EADDRNOTAVAIL);TestResolveAddressErrorandTestResolveAddressSuccesscoverresolveAddress.Before the fix the tests failed with:
Four mutations restoring the old returns were each caught.
Validation
go build ./...,go test ./... -count=1,go test -race ./diam/... -count=1,go vet ./...,staticcheck ./...andgolangci-lint run ./...all pass, andgofmt -l .is empty. Theexamples/middlewarebuild, vet and test pass.Fixes #68
Summary by cubic
Fixes SCTP listen and address resolution returning typed nil values on error.
Listen,MultistreamListen, andresolveAddressnow return a nil interface with the error, matchingnet.Listen, so callers that test the listener before the error or deferClose()no longer encounter a non-nil interface.Tests added for both listen functions across all SCTP and TCP networks, covering bind errors, invalid ports, and successful listens, plus
resolveAddresssuccess and error cases.Written for commit d6cc812. Summary will update on new commits.