Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move from net to netip #46

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Move from net to netip #46

wants to merge 3 commits into from

Conversation

monoidic
Copy link

This PR moves cidranger from using the IP address/prefix values from the "net" module in the Go standard libary to using the equivalents from the newer "net/netip" in the standard library. This allows for some code simplification:

  • no need to check for invalid underlying []byte lengths or nil pointers
  • netip.Prefix can be used directly as a map key in bruteranger as opposed to a net.IPNet's .String() value
  • address comparisons can be done directly with ==/!= as opposed to via bytes.Equal()

The tests were updated to use net/netip, and run successfully. TestInsertError and TestRemoveError were removed, as the error they are testing for is impossible to induce with net/netip

This is a fairly big update, and since it changes a good chunk of the API, likely warrants a version bump, though I am opening this PR to at least see if this is a change that you are interested in and if you have any changes in mind.
net/netip was introduced in 1.18, and, as 1.19 is out now, all officially supported versions of Go (the last two releases) support this, though applications and libraries making use of cidranger would need to be rewritten or at least add wrapper code to handle this, if this version is used.

@monoidic
Copy link
Author

It would also implicitly fix the issue outlined in PR #30 and issue #42, as net/netip handles IPv4-mapped IPv6 addresses far better than net.

@extemporalgenome
Copy link

This would be a breaking change, and thus would require a 2.x release.

If we're going to break compatibility, we might as well include other valuable changes, such as a switch from use of exposed interfaces (Ranger, RangerEntry) to using concrete types with generics. I believe the Ranger interface itself is just an anti-pattern, while RangerEntry is perhaps intended to allow associating data with a key, which generics would do in a cleaner and cheaper way since Go 1.18.

@extemporalgenome
Copy link

extemporalgenome commented Nov 30, 2022

For example, this PR is an incomplete/WIP adjustment of the API for generics building upon this PR.

@jhg03a jhg03a mentioned this pull request Feb 22, 2024
@jpillora
Copy link

FYI to those looking for this, I’m going to try out https://github.com/gaissmai/bart as a replacement, might be useful to others

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants