Re: macaddr 64 bit (EUI-64) datatype support

On Mon, Jan 9, 2017 at 1:45 PM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:
> Updated patch is attached.
>
I've a few comments about the patch.

+     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.

+#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.

+ if (!eight_byte_address)
+ {
+ d = 0xFF;
+ e = 0xFE;
+ }
You already have count variable. Just check (count != 8).

+ res *= 256 * 256;
+ res *= 256 * 256;
Bit shift operator can be used for this. For example: (res << 32).

-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.

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.


-- 
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com