Bugzilla – Bug 171
code bug in function (olsr)AgentImpl::MprComputation() about using std::vector::erase().
Last modified: 2008-07-01 13:32:28 UTC
there were code bug in function AgentImpl::MprComputation() about using std::vector.erase(). (In Version ns-3-dev) when i run the simple-point-to-point-olsr.cc, exceptions tracked. Commonly, std::vetor::erase() used like this example: /*****************************************************/ vector<int> arr; arr.push_back(8); for(vector<int>::iterator it=.begin(); it!=arr.end(); ) { if(* it == 8) { it = arr.erase(it); } else { ++it; } } /*****************************************************/ Also a patch file attached below.
Created attachment 129 [details] modify code about std::vector::erase() using in AgentImpl::MprComputation()
adding gustavo to CC list to get feedback.
First of all, please submit the patch as unified diff (diff -upd); I can't read this patch. Moreover, from the little I can read from the patch it looks like the patch is reversed?... Then I must confess I did not write most of the code, simply ported it from UM-OLSR (NS-2) to NS-3, so it is possible a bug or two prevailed... Incidentally, I did find some code rather strange, even added one or two FIXME comments. For example: void OlsrState::EraseTwoHopNeighborTuples (const Ipv4Address &neighborMainAddr, const Ipv4Address &twoHopNeighborAddr) { for (TwoHopNeighborSet::iterator it = m_twoHopNeighborSet.begin (); it != m_twoHopNeighborSet.end (); it++) { if (it->neighborMainAddr == neighborMainAddr && it->twoHopNeighborAddr == twoHopNeighborAddr) { it = m_twoHopNeighborSet.erase (it); it--; // FIXME: is this correct in the case 'it' pointed to the first element? m_modified = true; } } } But, since the code appeared to work, even when simulating with hundreds or thousands of nodes, when in doubt I chose to let it be. If you say it is giving segfaults then I'm all for fixing the code, as soon as you provided a proper patch. Thanks in advance.
(In reply to comment #3) > First of all, please submit the patch as unified diff (diff -upd); I can't read > this patch. Moreover, from the little I can read from the patch it looks like > the patch is reversed?... sorry about that. > Then I must confess I did not write most of the code, simply ported it from > UM-OLSR (NS-2) to NS-3, so it is possible a bug or two prevailed... > Incidentally, I did find some code rather strange, even added one or two FIXME > comments. For example: > void > OlsrState::EraseTwoHopNeighborTuples (const Ipv4Address &neighborMainAddr, > const Ipv4Address &twoHopNeighborAddr) > { > for (TwoHopNeighborSet::iterator it = m_twoHopNeighborSet.begin (); > it != m_twoHopNeighborSet.end (); it++) > { > if (it->neighborMainAddr == neighborMainAddr > && it->twoHopNeighborAddr == twoHopNeighborAddr) > { > it = m_twoHopNeighborSet.erase (it); > it--; // FIXME: is this correct in the case 'it' pointed to the first > element? here when the vector.size()== 1, 'it' still point to the first emlemnt, then "it--" will give exception. when i tracked ns-3-dev sample code, i does run abnormally. > m_modified = true; > } > } > } > But, since the code appeared to work, even when simulating with hundreds or > thousands of nodes, when in doubt I chose to let it be. If you say it is > giving segfaults then I'm all for fixing the code, as soon as you provided a > proper patch. Thanks in advance.
-> http://code.nsnam.org/ns-3-dev/rev/5d4ff983595b