6
84
Fork
You've already forked dns
39

Migrate from net.IP to netip.Addr #468

Merged
miekg merged 3 commits from aarongable/dns:netip-refactor into main 2025年12月12日 12:06:19 +01:00
Contributor
Copy link

Update the RR types which embed IP addresses (A, AAAA, and L32) to use netip.Addr instead of net.IP. This allows downstream packages which depend on this one to easily use this package no matter whether they use netip.Addr or net.IP, since conversion is a simple one-liner in one direction but not the other.

This small public interface change requires corresponding changes to the unpack, pack, scan, compare, and len functions. The changes to these library functions then require smaller changes to some of the higher-level code, such as dnstest and cmd/atomdns/handlers.

Update the RR types which embed IP addresses (A, AAAA, and L32) to use netip.Addr instead of net.IP. This allows downstream packages which depend on this one to easily use this package no matter whether they use netip.Addr or net.IP, since conversion is a simple one-liner in one direction but not the other. This small public interface change requires corresponding changes to the unpack, pack, scan, compare, and len functions. The changes to these library functions then require smaller changes to some of the higher-level code, such as dnstest and cmd/atomdns/handlers.
Migrate from net.IP to netip.Addr
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
7677f7edc0
@ -24,3 +23,3 @@
h.HandlerFunc(next).ServeDNS(context.TODO(),w,r)
ifx:=w.Msg.Answer[0].(*dns.A).A;slices.Compare(x,net.IP([]byte{127,0,0,1}))!=0{
ifx:=w.Msg.Answer[0].(*dns.A).A;x.Compare(netip.MustParseAddr("127.0.0.1"))!=0{
Owner
Copy link

this looks somewhat complex for the promised netip is so much easier... net.IP could be easily converted to netip we can use equal or some such?

this looks somewhat complex for the promised netip is so much easier... net.IP could be easily converted to netip we can use equal or some such?
Author
Contributor
Copy link

Yeah, using MustParseAddr (and the potential panics that it creates) doesn't make me happy either. But I think it's the right call here, because "127.0.0.1" is an obviously-correct hard-coded constant, which is exactly what MustParseAddr is intended for.

Yeah, using `MustParseAddr` (and the potential panics that it creates) doesn't make me happy either. But I think it's the right call here, because "127.0.0.1" is an obviously-correct hard-coded constant, which is exactly what MustParseAddr is intended for.
Author
Contributor
Copy link

d'oh, I misunderstood what you were saying. You're absolutely correct! I've changed this to use == instead of .Compare.

d'oh, I misunderstood what you were saying. You're absolutely correct! I've changed this to use == instead of .Compare.
aarongable marked this conversation as resolved
@ -22,3 +22,3 @@
family:=dnsutil.Family(w)
iffamily==1{
rr=&dns.A{Hdr:dns.Header{Name:r.Question[0].Header().Name,Class:dns.ClassINET},A:net.ParseIP(dnsutil.RemoteIP(w)).To4()}
rr=&dns.A{Hdr:dns.Header{Name:r.Question[0].Header().Name,Class:dns.ClassINET},A:netip.MustParseAddr(dnsutil.RemoteIP(w))}
Owner
Copy link

Must panics? This would introduce panic here, a nil A in old case is not good either, but will work

Must panics? This would introduce panic here, a `nil` A in old case is not good either, but will work
Author
Contributor
Copy link

Yeah, I noticed that the existing code is happy to populate the A/AAAA records with nil if the .To4() conversion fails. Using MustParseAddr was an easy shortcut to get similar behavior, but I can definitely do this The Right Way instead.

Yeah, I noticed that the existing code is happy to populate the A/AAAA records with `nil` if the `.To4()` conversion fails. Using `MustParseAddr` was an easy shortcut to get similar behavior, but I can definitely do this The Right Way instead.
aarongable marked this conversation as resolved
Owner
Copy link

some nits I guess
but very much worth while to merge, thanks for your work

some nits I guess but very much worth while to merge, thanks for your work
Avoid MustParseAddr for anything but hardcoded strings
Some checks are pending
ci/woodpecker/pr/build Pipeline is pending approval
3bcc7c498e
Use == instead of .Compare
Some checks are pending
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pull_request_closed/build Pipeline is pending approval
5554577db5
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
miekg/dns!468
Reference in a new issue
miekg/dns
No description provided.
Delete branch "aarongable/dns:netip-refactor"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?