Re: [PATCH 1/1] platform: extract common fields of IPv4/IPv6 addresses and routes to base struct



On Fri, 2014-04-04 at 16:24 -0500, Dan Williams wrote:
On Fri, 2014-04-04 at 23:02 +0200, Thomas Haller wrote:
On Fri, 2014-04-04 at 15:13 -0500, Dan Williams wrote:
On Thu, 2014-04-03 at 14:23 +0200, Thomas Haller wrote:
Especially the calculation of timestamps is identicall for addresses.
By creating a "base struct", we can use the same code for that, because
NMPlatformIP4Address and NMPlatformIP6Address can now both be treated as
NMPlatformIPAddress (and the same for routes).

Is the only reason for the #define of the common fields so that you
don't have to do another level of indirection?  It looks somewhat ugly
and my personal preference would be to just declare the base struct in
the functions you want to use it in and up-cast if you need the v4 or v6
version...  kinda like we do with objects.  So I certainly agree with
the principle, but lets see what other people say about the
implementation...


Hi Dan,

could you elaborate an what would be your preference? I don't
understand.

Sorry :)  I mean that I'm not wild about the #define of the common
fields,  but I'm OK with it if others think its fine.  Typically it be
something more like this:

typedef struct {
   NMPlatformIPAddress p;
   <ipv4 stuff>
} NMPlatformIP4Address;

typedef struct {
   NMPlatformIPAddress p;
   <ipv6 stuff>
} NMPlatformIP6Address;

void option1 (NMPlatformIP4Address *addr)
{
    NMPlatformIPAddress *parent = (NMPlatformIP4Address *) addr;
    addr->address = xxx;
}

void option2 (NMPlatformIP4Address *addr)
{
    addr->p.address = xxx;
}

which is more in line with kernel/glibc style I think.  It does mean a
bit more typing though.

Dan



I see, but the disadvantage is, that I would have to fixup *many*
occurrences in existing code. Also, it is more typing.

This way is also how libnl3 does it with the NLHDR_COMMON define (just
to compete with your glib role model :) )


Thomas

Attachment: signature.asc
Description: This is a digitally signed message part



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]