Bugzilla – Bug 678
InternetStackHelper::SetRoutingHelper does evil things
Last modified: 2009-10-02 01:50:33 UTC
void InternetStackHelper::SetRoutingHelper (const Ipv4RoutingHelper &routing) { m_routing = &routing; } It takes the address of an existing object, instead of copying it. It means I cannot do this: InternetStackHelper internet; internet.SetRoutingHelper (OlsrHelper()) The above code will crash (due to a pointer to a temporary stack allocated object), and an inexperienced programmer will have no idea why.
The basic problem here is that we seem to need a virtual constructor for a stack-allocated helper. Traditional ns-3 virtual constructor using CopyObject do not seem to apply if we want to keep these non-Ptr based. If we want to keep the API to be InternetStackHelper::SetRoutingHelper (const Ipv4RoutingHelper &routing); and allow the user to pass anonymous temporary objects, then, we could have a solution along these lines: class Ipv4RoutingHelper { public: virtual Ipv4RoutingHelper* Copy (void) const = 0; } class Ipv4ListRoutingHelper : public Ipv4RoutingHelper { public: Ipv4ListRoutingHelper (); Ipv4ListRoutingHelper (const Ipv4ListRoutingHelper &); Ipv4ListRoutingHelper &operator = (const Ipv4ListRoutingHelper &o); Ipv4ListRoutingHelper* Copy (void) const { return new Ipv4ListRoutingHelper (*this); } The problem with this is that it deviates from our memory management convention of not passing memory ownership across APIs using raw pointers. Am not seeing another clean alternative at the moment-- other ideas?
(In reply to comment #1) > class Ipv4ListRoutingHelper : public Ipv4RoutingHelper > { > public: > Ipv4ListRoutingHelper (); > Ipv4ListRoutingHelper (const Ipv4ListRoutingHelper &); > Ipv4ListRoutingHelper &operator = (const Ipv4ListRoutingHelper &o); > Ipv4ListRoutingHelper* Copy (void) const > { return new Ipv4ListRoutingHelper (*this); } > Am not seeing another clean alternative at the moment-- other ideas? This is exactly what I expected to do here.
Created attachment 613 [details] patch to fix This patch fixes this bug along the lines of the previous comment, for both IPv4 and IPv6. ./waf --valgrind --regression is clean.
Comment on attachment 613 [details] patch to fix >+Ipv4GlobalRoutingHelper & >+Ipv4GlobalRoutingHelper::operator = (const Ipv4GlobalRoutingHelper &o) >+{ >+ return *this; >+} Do you really need this ? Can't you just declare this operator private to disable it ? Anyway, the patch looks good to me: +1.
(In reply to comment #4) > (From update of attachment 613 [details]) > >+Ipv4GlobalRoutingHelper & > >+Ipv4GlobalRoutingHelper::operator = (const Ipv4GlobalRoutingHelper &o) > >+{ > >+ return *this; > >+} > > Do you really need this ? Can't you just declare this operator private to > disable it ? > > Anyway, the patch looks good to me: +1. > I don't need them, so I privatized them all. changeset a63513286aa0. Thanks for reviewing it.