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

Support GeoLite2 countryinfo data, use Text::CSV #2

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

Conversation

cgull
Copy link

@cgull cgull commented Jan 24, 2019

Here's a new PR with some further development. I think it fixes all the issues in #1, and a bug with GeoLite2 countryinfo parsing. I hadn't realized that parsing concatenated IPv4/IPv6 input was the way it's used for xtables, or that contrib/Makefile did that. There's also a commit to use Text::CSV for output, but that doesn't seem useful since we always quote all output values anyway, and it slows the script down slightly.

@mschmitt: if you take any/all of this, would you take entire commits, or ask me to refactor/improve them for you? After you took part of #1, it was somewhat painful to edit/rebase the rest of my work onto your new commit. It would have been much easier for me to split the commit myself and give it to you.

Also noted: I'm not sure what contrib/Makefile is there for, but it wasn't very useful for testing for me. It requires Linux, root access, and installed xtables and xtables geoIP data. It took a while to work this out, I'm using FreeBSD and iptables/xtables doesn't even exist there.

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.

1 participant