Thread: Conditional jump depends on unitialized value Valgrind

  1. #1
    Registered User
    Join Date
    Sep 2012
    Posts
    3

    Conditional jump depends on unitialized value Valgrind

    Hi, I am really lost here, I have airport class which should navigate planes, in its list to runways, with method move, theres a method prepare which changes the direction of flight to all planes, always before move is called, move just increments decrement x and y of plane in its list. But after calling two times in row airport->move(), I get screwed and I really dont know wheres the problem. Have I badly initiazed something? Iterator gets invalidated.

    Thanks for help.

    Valgrind Stacktrace
    Conditional jump or move depends on uninitialised value(s)
    ==26207== at 0x409601: plane::move() (in /home/xnovak11/Downloads/airport/main)
    ==26207== by 0x401FBD: airport::move() (in /home/xnovak11/Downloads/airport/main)
    ==26207== by 0x405FE1: io::start(std::istream&, std:stream&, std:stream&) (in /home/xnovak11/Downloads/airport/main)
    This is how I add planes into list in airport
    Code:
    list<plane*> planes;
        list<plane*>::const_iterator planeIterator;
        list<plane*>::iterator planeIteratorMoj;
    
    bool airport::addPlane(const plane& p)
    {
        for(planeIteratorMoj= planes.begin(); planeIteratorMoj!= planes.end(); planeIteratorMoj++)
        {
            if(p.getName()== (*planeIteratorMoj)->getName())
            {
                return false;
            }
        }
    
    
        plane *p1 = new plane(p);
        if(planes.empty())
        {
            planes.push_back(p1);
            if(planes.size()==1)
            {
                planeIterator = planes.begin();
            }
        }
        else
        {
            planes.push_back(p1);
            planeIterator = planes.begin();
        }
        return true;
    }
    This is the method where it fails.
    When I call it once, no problem, after second call I get instead of normal number in cout<<after move<< s1 i get like 8795456
    Dunno why.
    Code:
    void airport::move()
    {
    for(planeIteratorMoj = planes.begin(); planeIteratorMoj!= planes.end(); planeIteratorMoj++)
        {
            plane * p1 = (*planeIteratorMoj);
            int s,w;
            p1->getPosition(s,w);
            cout<<"before move function"<<s<<","<<w<< p1->getDirection()<<endl;
    
    
            if((*planeIteratorMoj)->move())
            {
                delete *planeIteratorMoj;
            }
    
    
            plane * p2 = (*planeIteratorMoj);
            int s1,w1;
            p2->getPosition(s1,w1);
            cout<<"after move function"<<s1<<","<<w1<< p2->getDirection()<<endl;
        }
    }
    This is plane class, the copy constructor and the move method.
    Code:
    enum DIRECTION
    {
        NORTH = 0,
        EAST = 1,
        SOUTH = 2,
        WEST = 3
    };
    
    
        string _name;
        int _x;
        int _y;
        DIRECTION _direction;
        bool _hasLanded;
        DIRECTION previous;
    
    
    
    plane::plane(const std::string& name, int x, int y, DIRECTION direction)
    {
        _name=name;
        _x=x;
        _y=y;
        _direction=direction;
        previous = direction;
    }
    
    
    /**
      Copy constructor
      @param other Airplane to make copy of
      */
    plane::plane(const plane& other)
    {
        _name=other.getName();
        other.getPosition(_x,_y);
        _direction=other.getDirection();
        _hasLanded = other._hasLanded;
        previous = _direction;
    }
    
    
    
    bool plane::move()
    {
    
    
        cout<<"plane move"<<_x<<","<<_y<<"   "<<getDirection()<<endl;
        switch(_direction)
        {
        case 0 :
            _y++;
            break;
    
    
        case 1 :
            _x++;
            break;
    
    
        case 2 :
            _y= _y -1;
            break;
    
    
        case 3 :
            _x=_x - 1;
            break;
        }
    
    
        if(_hasLanded)
        {
            return false;
        }
    
    
        return true;
    }
    Last edited by Ivan Novák; 01-10-2013 at 03:40 PM.

  2. #2
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    Look at this code in airport::move().

    Code:
            if((*planeIteratorMoj)->move())
            {
                delete *planeIteratorMoj;
            }
      
            plane * p2 = (*planeIteratorMoj);
            int s1,w1;
            p2->getPosition(s1,w1);
    Immediately after deleting *planeIteratorMoj, this code dereferences *planeIteratorMoj.



    Logically, I would query the notion of an airport being able to move. Airports are often geographically fixed entities, that can't move. Either the function airport::move() is poorly named, or the logic of your design is deficient. Or both.
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  3. #3
    Registered User
    Join Date
    Sep 2012
    Posts
    3
    p2 is there just for debbuging reasons,
    Airport has a list<plane*> and they have iterator planeIteratorMoj , so in the cycle i am calling function move in class plane, on instance of plane, if condition is true it means plane has landed and can be deleted.

    I want to ask you another thing, as I said I have list<plane*> where are correctly dynamically allocated instances of plane.
    If I have for cycle
    Code:
    for(planeIteratorMoj = planes.begin(); planeIteratorMoj!= planes.end(); planeIteratorMoj++)    {
            (*planeIteratorMoj)->move();
        }
    }
    As you can see function move in plane class is in my first topic, just increments internal variables of plane, are those changes correctly saved, so the plane inside a list has changed its position?

  4. #4
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    Ironically, the p2 may be there for debugging reasons, but what you're doing with it introduces a bug. If you are going to add code for debugging purposes, you need to be sure it works.

    Take out the references to p2 and the vector, and your code is equivalent to
    Code:
        delete x;
        x->getPosition(s1,w1);     // undefined behaviour here!
    As to your question, if all you're doing in the loop is changing the state of elements, that is fine. It is operations that change the vector itself (eg resizing it) that you need to be careful of, due to potential of invalidating iterators.

    Incidentally, don't rely on everyone reading your previous posts. With the number of people here, and number of topics, not everyone will read everything. And not everyone will bother to remember every small detail of your posts anyway. While I remember responding to your previous thread, I certainly haven't remembered the exquisite details of the code you posted in it.
    Last edited by grumpy; 01-11-2013 at 05:57 AM.
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  5. #5
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Why are you using new in the first place?
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  6. #6
    Registered User
    Join Date
    Mar 2010
    Posts
    583
    In your original example:

    Code:
            if((*planeIteratorMoj)->move())
            {
                delete *planeIteratorMoj;
            }
    You have a list of pointers to planes. This delete will delete the plane, but it won't remove the pointer from the list (i.e. list.size() will be the same).
    So on a subsquent call to airport::move(), the list of planes will have some pointers to freed memory.

    You probably want to delete the plane, and remove the pointer from the list. You can do that with list::erase list::erase - C++ Reference

    When you erase an element this way, the iterator to it becomes invalid, so you can't increment it. I'd suggest a while loop:
    Code:
       while (iter != thelist.end())
       {
          // do stuff
          if (condition to delete element)
              iter = thelist.erase(iter); 
          else
              iter++;
       }

    I'm surprised valgrind didn't give you a specific diagnostic about this: saying freed memory had been accessed.

    If you compile your code with debug information (-g), then valgrind can tell you the source line that caused a problem.

    This:

    Conditional jump or move depends on uninitialised value(s)
    ==26207== at 0x409601: plane::move() (in /home/xnovak11/Downloads/airport/main)
    I'd guess this is:
    Code:
        if(_hasLanded)
        {
            return false;
        }
    You don't seem to have set _hasLanded anywhere, at least not in the code you've posted.

    Also, you said:
    if condition is true it means plane has landed and can be deleted.
    which is the opposite of what this code is doing.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. on what program execution time depends upon
    By suryak in forum C Programming
    Replies: 2
    Last Post: 05-19-2011, 01:03 AM
  2. Replies: 5
    Last Post: 01-15-2011, 12:31 PM
  3. Replies: 3
    Last Post: 05-23-2010, 07:32 AM
  4. vector::size() unitialized at creation
    By VirtualAce in forum C++ Programming
    Replies: 9
    Last Post: 03-08-2008, 01:48 AM
  5. Checking unitialized class pointer
    By cunnus88 in forum C++ Programming
    Replies: 7
    Last Post: 11-16-2005, 05:36 PM