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

From: Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
To: Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>
Cc: Vitaly Burovoy <vitaly(dot)burovoy(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, Shay Rojansky <roji(at)roji(dot)org>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Julien Rouhaud <julien(dot)rouhaud(at)dalibo(dot)com>, Craig Ringer <craig(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: macaddr 64 bit (EUI-64) datatype support
Date: 2017-01-19 02:16:05
Message-ID: CAJrrPGdzKN1F+p8oPpDZ8WRn-+vbS3BJFf8GnBv5PCfwJ8x9kw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Sat, Jan 14, 2017 at 6:28 PM, Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>
wrote:

> On Mon, Jan 9, 2017 at 1:45 PM, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)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

Attachment Content-Type Size
mac_eui64_support_6.patch application/octet-stream 49.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2017-01-19 02:29:38 Re: Patch to implement pg_current_logfile() function
Previous Message Tom Lane 2017-01-19 01:38:11 Re: Re: Clarifying "server starting" messaging in pg_ctl start without --wait