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

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