On Sat, Jan 14, 2017 at 6:28 PM, Kuntal Ghosh <kuntalghosh.2...@gmail.com> wrote:
> On Mon, Jan 9, 2017 at 1:45 PM, Haribabu Kommi <kommi.harib...@gmail.com>
> wrote:
> > Updated patch is attached.
> >
> I've a few comments about the patch.
>
Thanks for the review.
+ This type can accept both 6 and 8 bytes length MAC addresses.
> A 6 bytes length MAC address is internally converted to 8 bytes. We
> should include this in the docs. Because output is always 8 bytes.
> Otherwise, a user may be surprised with the output.
>
updated accordingly.
> +#define hibits(addr) \
> + ((unsigned long)(((addr)->a<<24)|((addr)->b<<16)|((addr)->c<<8)|((addr)
> ->d)))
> +
> +#define lobits(addr) \
> + ((unsigned long)(((addr)->e<<24)|((addr)->f<<16)|((addr)->g<<8)|((addr)
> ->h)))
> +
> There should be some spacing.
>
corrected.
+ if (!eight_byte_address)
> + {
> + d = 0xFF;
> + e = 0xFE;
> + }
> You already have count variable. Just check (count != 8).
>
Changed.
> + res *= 256 * 256;
> + res *= 256 * 256;
> Bit shift operator can be used for this. For example: (res << 32).
>
Changed by adding a typecast, because res is a double datatype value.
> -DATA(insert ( 403 macaddr_ops PGNSP PGUID 1984 829 t 0 ));
> -DATA(insert ( 405 macaddr_ops PGNSP PGUID 1985 829 t 0 ));
> +DATA(insert ( 403 macaddr_ops PGNSP PGUID 1984 829 t 0 ));
> +DATA(insert ( 405 macaddr_ops PGNSP PGUID 1985 829 t 0 ));
> This is unnecessary I guess.
>
Corrected.
> There was some whitespace error while applying the patch. Also, there
> are some spacing inconsistencies in the comments. I've not tested with
> this patch. I'll let you know once I'm done testing.
>
Corrected.
Updated patch attached.
Regards,
Hari Babu
Fujitsu Australia
mac_eui64_support_6.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
