|
Bugzilla – Full Text Bug Listing |
| Summary: | code bug in function (olsr)AgentImpl::MprComputation() about using std::vector::erase(). | ||
|---|---|---|---|
| Product: | ns-3 | Reporter: | Liu Jian <liujatp> |
| Component: | routing | Assignee: | ns-bugs <ns-bugs> |
| Status: | RESOLVED FIXED | ||
| Severity: | minor | CC: | gjcarneiro, liujatp |
| Priority: | P3 | Keywords: | bug |
| Version: | pre-release | ||
| Hardware: | All | ||
| OS: | Windows | ||
| Attachments: | modify code about std::vector::erase() using in AgentImpl::MprComputation() | ||
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. |
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.