[Xorp-hackers] Duplicated code in RIP

[Marko Zec]

On Thursday 10 November 2005 08:14, Mark Doll wrote:
> 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).


I think you are right, and basically like the proposal to further reduce 
the specialization.  Unfortunately RIP has other more fundamental 
problems that have to be dealt with first - it listens on a single 
wildcard socket instead of multiple per-interface ones.  Once this gets 
cleaned up I agree that templatizing the XRL socket interface code 
might be worth a try.

Cheers,

Marko