On Mon, 2014-04-07 at 14:10 -0400, Dan Winship wrote:
On 04/04/2014 05:46 PM, Thomas Haller wrote: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: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...I see, but the disadvantage is, that I would have to fixup *many* occurrences in existing code. Also, it is more typing.But many of those existing occurrences involve separate IPv4 and IPv6 functions / codepaths where the actual behavior is exactly the same and ought to be merged into a single NMPlatformIPAddress codepath anyway... -- Dan
I would say that many occurrences are no candidates for refactoring: $ git grep '\(->\|\.\)\<lifetime\>' (which does not mean, that it isn't useful in several cases!) I think, there are several reasons for the *_COMMON define: * it would need more typing for no benefit: - addr.lifetime = lifetime; + addr.common.lifetime = lifetime; or + ((NMPlatformIPAddress *) &addr)->lifetime = lifetime; * I have another version of the patch, that combines NMPlatformLink, NMPlatform*Address and NMPlatform*Route with: +#define __NMPlatformObject_COMMON \ + int ifindex; \ + ; that would mean: - addr.ifindex = ifindex; - addr.lifetime = lifetime; + addr.common_addr.common.ifindex = ifindex; + addr.common_addr.lifetime = lifetime; or + ((NMPlatformObject *) &addr)->ifindex = ifindex; + ((NMPlatformIPAddress *) &addr)->lifetime = lifetime; * I don't want to touch existing/working code now (even if it's trivially fixing compile errors). Thomas PS: yes, I have an actual usecase for this patch
Attachment:
signature.asc
Description: This is a digitally signed message part