Bug 169

Created attachment 128 [details]
modify iterator j

avoid j overflow

adding craig to CC list to get feedback from him.

It would be very helpful if you mention the following in your bug reports:

1) The context -- i.e., what was going on when the bug was triggered;
2) What incorrect behavior happened as a result of the bug.

If you provide a proposed fix, it would be nice if you added,

1) What caused the incorrect behavior;
2) What you did to fix it.

(In reply to comment #3)
> It would be very helpful if you mention the following in your bug reports:
> 1) The context -- i.e., what was going on when the bug was triggered;
> 2) What incorrect behavior happened as a result of the bug.
> If you provide a proposed fix, it would be nice if you added,

sorry about that.i just begin to learn ns3.
 
> 1) What caused the incorrect behavior;

in function:
Ipv4AddressGeneratorImpl::AddAllocated (const Ipv4Address address)
line 272, code:

std::list<Entry>::iterator i, j;  
for (i = m_entries.begin (), j = m_entries.begin (), ++j; 
i != m_entries.end (); ++i, ++j) 
 
here, if m_entries.size == 0,  "++j" will trigger exception

{    
 NS_LOG_LOGIC ("examine entry: " << Ipv4Address ((*i).addrLow) 
        " to " << Ipv4Address ((*i).addrHigh)); 
......//other codes

here if 'i' point to the last element, then j point to m_entries.end(),then in next cycle, " ++i, ++j", "++j" will also trigger exception
}

> 2) What you did to fix it.
so, replace initial "++j" with "j = i", 
in each cycle:let "j = i"; before j used, "j++" added

line 303:
........
     if (addr == (*i).addrHigh + 1) 
        {              
         j ++; 
         //added herefor j point to next element after i

          if (j != m_entries.end ())
          { 
              if (addr == (*j).addrLow) 
........

I don't mean to be a pain, and I'm not arguing that there is no bug; but you really need to be more specific when reporting a bug.  You seem to be saying that if I compile and run the following program,

#include "ns3/ipv4-address-generator.h"

using namespace ns3;

  int
main (int argc, char *argv[])
{
  Ipv4AddressGenerator::AddAllocated ("1.2.3.4");
}

It will fail.  This code has been in use for some time, so this clearly isn't the case.  You must be running on a system that has a different stl implementation than everyone else.  This is an important tidbit to avoid future problems like those you've reported regarding the library during our testing process.  There are larger issues than just the j++ here.  Can you provide some info about your environment as well?

thanks for your advice about reporting a bug, it was really helpful to me.

About the std::list<>iterator j++ in Ipv4AddressGenerator::AddAllocated(), what i want to spot is not this function's context but the way how j ++ use.
a simple example here:
/********************************************************/
#include <list>
#include <iostream>
int main (int argc, char *argv[])
{
    std::list<int>data;
    std::list<int>::iterator i,j;

    data.push_back(1);

    for (i = data.begin (), j = data.begin (), ++j; i != data.end (); ++i, ++j) 
    {
        std::cout << "element i = " << *i <<"; element j = " <<  *j <<std::endl;
    }
}
/********************************************************/
here the output<in FC8>:element i = 1, element j = 399188.
the code will not cause segment fault, but get wrong result.

So in Ipv4AddressGenerator::AddAllocated(),  it was true that it run normally for some time. But when i == m_entries.end(), j ++ point to next uncertain element. it may be will cause wrong running result,or maybe NOT. That was why i report this.

Okay.  I think I understand now.  Did you find this bug by inspecting the code; and not by debugging an error on some target system?