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

[Dan Williams <dcbw redhat com>]

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

Dan

> Signed-off-by: Thomas Haller <thaller redhat com>
> ---
>  I think introducing this "base class" could help us to put similar
>  code for IPv4 and IPv6 types into a function. Especially handling of
>  the timestamps/expiration.
>
>  src/platform/nm-platform.h | 50 +++++++++++++++++++++++++++++-----------------
>  1 file changed, 32 insertions(+), 18 deletions(-)
>
> diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h
> index 356af1f..dd56977 100644
> --- a/src/platform/nm-platform.h
> +++ b/src/platform/nm-platform.h
> @@ -143,19 +143,32 @@ typedef enum {
>       NM_PLATFORM_SOURCE_USER,
>  } NMPlatformSource;
>  
> +#define __NMPlatformIPAddress_COMMON \
> +     int ifindex; \
> +     NMPlatformSource source; \
> +     guint32 timestamp;  /* nm_utils_get_monotonic_timestamp_s() */ \
> +     guint32 lifetime;   /* seconds */ \
> +     guint32 preferred;  /* seconds */ \
> +     ;
> +
> +/**
> + * NMPlatformIPAddress:
> + *
> + * Common parts of NMPlatformIP4Address and NMPlatformIP6Address.
> + **/
> +typedef struct {
> +     __NMPlatformIPAddress_COMMON
> +} NMPlatformIPAddress;
> +
>  /**
>   * NMPlatformIP4Address:
>   * @timestamp: timestamp as returned by nm_utils_get_monotonic_timestamp_s()
>   **/
>  typedef struct {
> -     int ifindex;
> -     NMPlatformSource source;
> +     __NMPlatformIPAddress_COMMON
>       in_addr_t address;
>       in_addr_t peer_address;  /* PTP peer address */
>       int plen;
> -     guint32 timestamp;
> -     guint32 lifetime;   /* seconds */
> -     guint32 preferred;  /* seconds */
>       char label[IFNAMSIZ];
>  } NMPlatformIP4Address;
>  
> @@ -164,37 +177,38 @@ typedef struct {
>   * @timestamp: timestamp as returned by nm_utils_get_monotonic_timestamp_s()
>   **/
>  typedef struct {
> -     int ifindex;
> -     NMPlatformSource source;
> +     __NMPlatformIPAddress_COMMON
>       struct in6_addr address;
>       struct in6_addr peer_address;
>       int plen;
> -     guint32 timestamp;  /* seconds */
> -     guint32 lifetime;   /* seconds */
> -     guint32 preferred;
>       guint flags; /* ifa_flags from <linux/if_addr.h>, field type "unsigned int" is as used in 
> rtnl_addr_get_flags. */
>  } NMPlatformIP6Address;
>  
>  #define NM_PLATFORM_ROUTE_METRIC_DEFAULT 1024
>  
> +#define __NMPlatformIPRoute_COMMON \
> +     int ifindex; \
> +     NMPlatformSource source; \
> +     guint metric; \
> +     guint mss; \
> +     ;
> +
>  typedef struct {
> -     int ifindex;
> -     NMPlatformSource source;
> +     __NMPlatformIPRoute_COMMON
> +} NMPlatformIPRoute;
> +
> +typedef struct {
> +     __NMPlatformIPRoute_COMMON
>       in_addr_t network;
>       int plen;
>       in_addr_t gateway;
> -     guint metric;
> -     guint mss;
>  } NMPlatformIP4Route;
>  
>  typedef struct {
> -     int ifindex;
> -     NMPlatformSource source;
> +     __NMPlatformIPRoute_COMMON
>       struct in6_addr network;
>       int plen;
>       struct in6_addr gateway;
> -     guint metric;
> -     guint mss;
>  } NMPlatformIP6Route;
>  
>  typedef struct {