[Xorp-hackers] Duplicated code in RIP

Mark Doll doll@tm.uka.de
Thu, 10 Nov 2005 08:14:33 +0100


Hi Marko!

Marko Zec wrote:
> On Wednesday 09 November 2005 18:06, Mark Doll wrote:
> 
>>I wonder why some member functions of class XrlPortIO<A> (i.e.
>>request_socket_server, see file ~/rip/xlr_port_io.cc) are not
>>generally defined using template parameter 'A' but are instead
>>defined two times as specializations for 'IPv4' and 'IPv6', although
>>the specializations only differ in constants and types, differences
>>that are otherwise handled with class RIP_AF_CONSTANTS<A> throughout
>>RIP.
> 
> 
> Another difference is that RIP sends / receives packets on IPv4 sockets, 
> while RIPng uses IPv6 sockets.  Different socket types -> different XRL 
> interfaces that need to be used, thus the need for specialization 
> (socket4 vs socket6) in certain member functions.

No, the interfaces socket4 and socket6 are identical except the address type (ipv4 vs. ipv6) and one minor naming difference in a socket option ("multicast_ttl" vs. "multicast_hops", just try a 'diff xrl/interfaces/socket{4,6}.xif').

So the code to access these two interfaces stays the same and can be programmed generically using templates. The code only has to declare the object used to access these interfaces to be of a different type (XrlSocket4V0p1Client vs. XrlSocket6V0p1Client). This and the different naming of the ttl socket option can be easily handled with RIP_AF_CONSTANTS<A> (which already handles the different port, group, default route and version of RIP and RIPng, cf. ~/rip/constants.hh).

I just wondered why they just stop in the middle and don't handle the remaining two differences with RIP_AF_CONSTANTS<A>, too. Maybe there's a good reason I'm not aware of.

Mark.

P.S.: Since a type is not a constant, it might make (semantically) more sense not to use RIP_AF_CONSTANTS<A> but to define another object, something like this:

template<class> struct XrlSocketClientType;
template<> struct XrlSocketClientType<IPv4> { typedef XrlSocket4V0p1Client type; };
template<> struct XrlSocketClientType<IPv6> { typedef XrlSocket6V0p1Client type; };

template <class A>
class XrlPortIO : ...
{
public:
    typedef typename XrlSocketClientType<A>::type XrlSocketClient;
...

and then use 'XrlSocketClient' in the functions instead of 'XrlSocket4V0p1Client' and 'XrlSocket6V0p1Client'. This has the convenient side effect that you only have to change one typedef in case the version of the socket interfaces changes in the future (and the changes in the interface don't affect RIP's usage of it).