On Thu, 3 Mar 2005, [ISO-8859-1] Xu�n Baldauf wrote:
The "#if ENABLE_IPV6" conditions are to provide a more smooth transition to enabled IPv6 support. The idea is: When "ENABLE_IPV6==0", then everything is as before. But when ENABLE_IPV6==1, every variable and every struct which refers IPv4 adresses (the old way) just do not exist.
I know, and this is what I dislike.
By using the ifdefs, the IPv6 code can not be introduced into HEAD in a meaningful manner as it is bound to bitrot pretty quickly unless all developers switches to build their Squids IPv6 enabled, as even trivial errors won't be detected compile time. Also it considerably obfuscates the code and makes maintenance a burden.
Yes, doing it proper from start may requires a little bigger initial effort, but only marginally so I think (not counting the effort you have already put into doing it..). In both cases a new type for ipv6 needs to be introduced, and in both cases the compiler will tell you which lines is affected due to the type change.
This approach (of breaking compileability with intent) has the side effect that, until IPv6 support has been provided nearly completely, the modified code would not compile. Thus, the modification is encapsulated by "#if ENABLE_IPV6" to protect uninterested developers to be forced to fix IPv6 generated compiler error messages.
The same can be acheived by providing a typecast operator to the old address types only when IPv6 is not enabled and use this in the glue between converted and not yet converted code.
In addition, this part of the discussion is only interesting when it comes to merging the IPv6 support into HEAD, not the initial efforts.
lacking, just subclassing an "Address" class to an "IPv6Address" class is not possible (the "Address" class is missing... ;-)).
Only because something is missing does not say it should stay that way. It should not stay that way.
HEAD tree would be able to create a failed sync between "HEAD" and "squid-ipv6" branches, because a separate "squid-ipv6" branch would not exist, the "#if ENABLE_IPV6" sections are the branch.)
The current #if ENABLE_IPV6 won't make it into HEAD for the reasons outlined above. This style of programming is bound to stay in a separate branch until done proper.
Having "#if ENABLE_IPV6" statements should be only temporary. When there are no IPv6 support compile error messages anymore, ENABLE_IPV6==1 should become the default. After some more testing, all
#if ENABLE_IPV6 /* new code */ #else /* old code */ #endif
sections in the sourcecode shall be replaced by
/* new code */
so the "#if ENABLE_IPV6" statements will then be removed, too.
This will be possible, because "ENABLE_IPV6" does not really mean that we are going away from IPv4, but just that the software will be IP version neutral, explicitly supporting both IPv4 and IPv6.
Not all OS:es where Squid builds is IPv6 enabled which complicates matters somewhat. By introducing a new proper address class this dependency gets isolated down to the address class, keeping the bulk of the code clean.
Regards Henrik
