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

dotted-quad support for IPv4 subnet masks #30

Open
abraxxa opened this issue Jan 31, 2022 · 9 comments
Open

dotted-quad support for IPv4 subnet masks #30

abraxxa opened this issue Jan 31, 2022 · 9 comments

Comments

@abraxxa
Copy link
Contributor

abraxxa commented Jan 31, 2022

I have to work with a product that returns IPv4 networks with the subnet mask in dotted-quad notation and need to parse those.
Can we add support for in- and output of this format without breaking things?

Thanks!

@oschwald
Copy link
Member

That seems possible. A PR that did this with appropriate tests and documentation would likely be accepted.

@abraxxa
Copy link
Contributor Author

abraxxa commented Jan 31, 2022

I was already able to use use Net::Works::Util::_integer_address_to_string($net->_subnet_integer) for generting the dotted-quad output.
The guesswork in new_from_string is what worries me.

@oschwald
Copy link
Member

Yeah, I could see guessing potentially leading to confusion in some cases. I suppose an additional parameter, e.g., dotted_decimal_mask => 1 or something could work.

@abraxxa
Copy link
Contributor Author

abraxxa commented Feb 21, 2022

@oschwald Before I start with this pull-request, can you be so kind to look at my fork which uses binary representation instead of integer/Math::Int128?
My goal was to get rid of Math::Int128 as far as I can remember and provide an API for binary values to store those in a SQL database that has no Int128 or IP address/network support (Oracle) using its RAW database which IO is in uppercase hex.
Thanks!

@abraxxa
Copy link
Contributor Author

abraxxa commented Feb 21, 2022

I've rebased my fork with my commits from 2014 against current main and called it main, so my original modifications are also viewable in the still existing master branch.

@oschwald
Copy link
Member

@abraxxa, sure, do you have a link to what you want me to review?

@abraxxa
Copy link
Contributor Author

abraxxa commented Feb 23, 2022

The main branch in my fork please.

@abraxxa
Copy link
Contributor Author

abraxxa commented May 4, 2022

@oschwald friendly reminder ;-)

@oschwald
Copy link
Member

oschwald commented Jun 2, 2022

Sorry for the delay. At a high level, most of the changes seem good. It will be easier to review once there is a PR that I can comment on.

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

No branches or pull requests

2 participants