On Mon, Mar 13, 2017 at 6:38 AM, Stephen Frost <sfrost@snowman.net> wrote:
Greetings, * Stephen Frost (sfrost@snowman.net) wrote: > * Haribabu Kommi (kommi.haribabu@gmail.com) wrote: > > On Wed, Feb 1, 2017 at 6:27 AM, Vitaly Burovoy <vitaly.burovoy@gmail.com> wrote: > > > The new status of this patch is: Ready for Committer > > > > Thanks for the review. > > I've started taking a look at this with an eye towards committing it > soon.
I've spent a good bit of time going over this, possibly even more than it was worth, but hopefully we'll see people making use of this data type with PG10 and as more IPv6 deployment happens.
Thanks for the review.
Of particular note, I rewrote macaddr8_in to not use sscanf(). sscanf(), at least on my system, would accept negative values even for '%2x', leading to slightly odd errors with certain inputs, including with our existing macaddr type:
=# select '00-203040506'::macaddr; ERROR: invalid octet value in "macaddr" value: "00-203040506" LINE 1: select '00-203040506'::macaddr;
With my rewrite, the macaddr8 type will throw a clearer (in my view, at least) error:
=# select '00-203040506'::macaddr8; ERROR: invalid input syntax for type macaddr8: "00-203040506" LINE 1: select '00-203040506'::macaddr8;
One other point is that the previously disallowed format with just two colons ('0800:2b01:0203') is now allowed. Given that both the two dot format ('0800.2b01.0203') and the two dash format ('0800-2b01-0203') were accepted, this seemed alright to me. Is there really a good reason to disallow the two colon format?
No. There is no special reason to disallow.
The rewrite of macaddr8_in will allow all possible combinations of spacers.
I also thought about what we expect the usage of macaddr8 to be and realized that we should really have a function to help go from EUI-48 to the IPv6 Modified EUI-64 format, since users will almost certainly want to do exactly that. As such, I added a macaddr8_set7bit() function which will perform the EUI-64 -> Modified EUI-64 change (which is just setting the 7th bit) and added associated documentation and reasoning for why that function exists.