-
Problem with std::vector
I've run into an error that I can't explain. Here is some output from valgrind:
Code:
==573== Invalid read of size 1
==573== at 0x40273E8: memmove (mc_replace_strmem.c:517)
==573== by 0x8050CAC: Merchandise** std::__copy_move<false, true, std::random_access_iterator_tag>::__copy_m<Merchandise*>(Merchandise* const*, Merchandise* const*, Merchandise**) (stl_algobase.h:377)
==573== by 0x8050CE6: Merchandise** std::__copy_move_a<false, Merchandise* const*, Merchandise**>(Merchandise* const*, Merchandise* const*, Merchandise**) (stl_algobase.h:396)
==573== by 0x8050D25: Merchandise** std::__copy_move_a2<false, __gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**>(__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, __gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**) (stl_algobase.h:435)
==573== by 0x8050D75: Merchandise** std::copy<__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**>(__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, __gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**) (stl_algobase.h:466)
==573== by 0x8050D96: Merchandise** std::__uninitialized_copy<true>::uninitialized_copy<__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**>(__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, __gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**) (stl_uninitialized.h:98)
==573== by 0x8050DB7: Merchandise** std::uninitialized_copy<__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**>(__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, __gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**) (stl_uninitialized.h:122)
==573== by 0x8050DD8: Merchandise** std::__uninitialized_copy_a<__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**, Merchandise*>(__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, __gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**, std::allocator<Merchandise*>&) (stl_uninitialized.h:262)
==573== by 0x80516EC: std::vector<Merchandise*, std::allocator<Merchandise*> >::vector(std::vector<Merchandise*, std::allocator<Merchandise*> > const&) (stl_vector.h:249)
==573== by 0x804E47A: Station::findProduct(int) (Station.cpp:99)
==573== by 0x804D62F: Sector::findLowestSeller(int) (Sector.cpp:86)
==573== by 0x8052747: Ship::generateNewOrders() (Ship.cpp:519)
==573== Address 0x42ec8f7 is 1 bytes before a block of size 8 alloc'd
==573== at 0x402573E: operator new(unsigned) (vg_replace_malloc.c:224)
==573== by 0x8051211: __gnu_cxx::new_allocator<Merchandise*>::allocate(unsigned, void const*) (new_allocator.h:92)
==573== by 0x805123B: std::_Vector_base<Merchandise*, std::allocator<Merchandise*> >::_M_allocate(unsigned) (stl_vector.h:144)
==573== by 0x80517FE: std::vector<Merchandise*, std::allocator<Merchandise*> >::_M_insert_aux(__gnu_cxx::__normal_iterator<Merchandise**, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise* const&) (vector.tcc:308)
==573== by 0x805198E: std::vector<Merchandise*, std::allocator<Merchandise*> >::push_back(Merchandise* const&) (stl_vector.h:694)
==573== by 0x8052D04: Ship::addCargo(int, int) (Ship.cpp:444)
==573== by 0x8052E00: Ship::buyCargo(int, int) (Ship.cpp:343)
==573== by 0x80533AA: Ship::tick() (Ship.cpp:188)
==573== by 0x804D34F: Sector::tick() (Sector.cpp:63)
==573== by 0x8056E28: main (main.cpp:77)
==573==
==573== Invalid read of size 1
==573== at 0x40273FA: memmove (mc_replace_strmem.c:517)
==573== by 0x8050CAC: Merchandise** std::__copy_move<false, true, std::random_access_iterator_tag>::__copy_m<Merchandise*>(Merchandise* const*, Merchandise* const*, Merchandise**) (stl_algobase.h:377)
==573== by 0x8050CE6: Merchandise** std::__copy_move_a<false, Merchandise* const*, Merchandise**>(Merchandise* const*, Merchandise* const*, Merchandise**) (stl_algobase.h:396)
==573== by 0x8050D25: Merchandise** std::__copy_move_a2<false, __gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**>(__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, __gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**) (stl_algobase.h:435)
==573== by 0x8050D75: Merchandise** std::copy<__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**>(__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, __gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**) (stl_algobase.h:466)
==573== by 0x8050D96: Merchandise** std::__uninitialized_copy<true>::uninitialized_copy<__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**>(__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, __gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**) (stl_uninitialized.h:98)
==573== by 0x8050DB7: Merchandise** std::uninitialized_copy<__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**>(__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, __gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**) (stl_uninitialized.h:122)
==573== by 0x8050DD8: Merchandise** std::__uninitialized_copy_a<__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**, Merchandise*>(__gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, __gnu_cxx::__normal_iterator<Merchandise* const*, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise**, std::allocator<Merchandise*>&) (stl_uninitialized.h:262)
==573== by 0x80516EC: std::vector<Merchandise*, std::allocator<Merchandise*> >::vector(std::vector<Merchandise*, std::allocator<Merchandise*> > const&) (stl_vector.h:249)
==573== by 0x804E47A: Station::findProduct(int) (Station.cpp:99)
==573== by 0x804D62F: Sector::findLowestSeller(int) (Sector.cpp:86)
==573== by 0x8052747: Ship::generateNewOrders() (Ship.cpp:519)
==573== Address 0x42ec8f5 is 3 bytes before a block of size 8 alloc'd
==573== at 0x402573E: operator new(unsigned) (vg_replace_malloc.c:224)
==573== by 0x8051211: __gnu_cxx::new_allocator<Merchandise*>::allocate(unsigned, void const*) (new_allocator.h:92)
==573== by 0x805123B: std::_Vector_base<Merchandise*, std::allocator<Merchandise*> >::_M_allocate(unsigned) (stl_vector.h:144)
==573== by 0x80517FE: std::vector<Merchandise*, std::allocator<Merchandise*> >::_M_insert_aux(__gnu_cxx::__normal_iterator<Merchandise**, std::vector<Merchandise*, std::allocator<Merchandise*> > >, Merchandise* const&) (vector.tcc:308)
==573== by 0x805198E: std::vector<Merchandise*, std::allocator<Merchandise*> >::push_back(Merchandise* const&) (stl_vector.h:694)
==573== by 0x8052D04: Ship::addCargo(int, int) (Ship.cpp:444)
==573== by 0x8052E00: Ship::buyCargo(int, int) (Ship.cpp:343)
==573== by 0x80533AA: Ship::tick() (Ship.cpp:188)
==573== by 0x804D34F: Sector::tick() (Sector.cpp:63)
==573== by 0x8056E28: main (main.cpp:77)
Entering Station::findMerchandise(int, std::vector<Merchandise*>)
==573==
==573== Invalid read of size 4
==573== at 0x804E00A: Station::findMerchandise(int, std::vector<Merchandise*, std::allocator<Merchandise*> >) (Station.cpp:81)
==573== by 0x804E493: Station::findProduct(int) (Station.cpp:99)
==573== by 0x804D62F: Sector::findLowestSeller(int) (Sector.cpp:86)
==573== by 0x8052747: Ship::generateNewOrders() (Ship.cpp:519)
==573== by 0x80532D8: Ship::tick() (Ship.cpp:165)
==573== by 0x804D34F: Sector::tick() (Sector.cpp:63)
==573== by 0x8056E28: main (main.cpp:77)
==573== Address 0x6e617254 is not stack'd, malloc'd or (recently) free'd
==573==
==573== Process terminating with default action of signal 11 (SIGSEGV)
==573== Access not within mapped region at address 0x6E617254
==573== at 0x804E00A: Station::findMerchandise(int, std::vector<Merchandise*, std::allocator<Merchandise*> >) (Station.cpp:81)
==573== by 0x804E493: Station::findProduct(int) (Station.cpp:99)
==573== by 0x804D62F: Sector::findLowestSeller(int) (Sector.cpp:86)
==573== by 0x8052747: Ship::generateNewOrders() (Ship.cpp:519)
==573== by 0x80532D8: Ship::tick() (Ship.cpp:165)
==573== by 0x804D34F: Sector::tick() (Sector.cpp:63)
==573== by 0x8056E28: main (main.cpp:77)
Here is Station::findProduct
Code:
Merchandise *Station::findProduct(int m_type) {
if(this->sector->galaxy->verbosity >= VERBOSITY_ENTER_EXIT) {
std::cerr << "Entering Station::findProduct(int)" << std::endl;
}
Merchandise *retval = this->findMerchandise(m_type, this->products); // this is line 99
if(this->sector->galaxy->verbosity >= VERBOSITY_ENTER_EXIT) {
std::cerr << "Exiting Station::findProduct(int)" << std::endl;
}
return retval;
}
Here is Station::findMerchandise
Code:
Merchandise *Station::findMerchandise(int m_type, std::vector<Merchandise*> m_vec) {
if(this->sector->galaxy->verbosity >= VERBOSITY_ENTER_EXIT) {
std::cerr << "Entering Station::findMerchandise(int, std::vector<Merchandise*>)" << std::endl;
}
Merchandise *retval = NULL;
std::vector<Merchandise*>::iterator iter;
for(iter = m_vec.begin(); iter != m_vec.end(); iter++) {
if ((*iter)->type == m_type) { // this is line 81
retval = *iter;
break;
}
}
if(this->sector->galaxy->verbosity >= VERBOSITY_ENTER_EXIT) {
std::cerr << "Exiting Station::findMerchandise(int, std::vector<Merchandise*>)" << std::endl;
}
return retval;
}
And here is Station.h:
Code:
#ifndef STATION_H
#define STATION_H
#include <vector>
#include <string>
#include "Entity.h"
#include "StationTypes.h"
#include "Ship.h"
struct Merchandise;
struct StationInfo;
class Station : public Entity {
private:
Merchandise *findMerchandise(int m_type, std::vector<Merchandise*> m_vec);
void updatePrice(Merchandise *m);
void updatePrices();
int countdown;
bool checkInventory();
StationInfo* station_info;
protected:
public:
int station_type;
Station(std::string name, int station_type, Sector* sector, Point3D position);
~Station();
std::string name;
std::vector<Merchandise*> products;
std::vector<Merchandise*> resources;
std::vector<Ship*> docked_ships;
bool buyProducts(int m_type, int amt);
bool sellResources(int m_type, int amt);
int funds;
Merchandise* findMerchandiseAll(int m_type);
Merchandise *findProduct(int m_type);
Merchandise *findResource(int m_type);
void tick();
std::string statusString();
bool dock(Ship *s);
bool undock(Ship *s);
};
#endif
I used to get an std::bad_alloc exception. It also happened at this same part of the code, but I'm not sure what changed to make that go away. There is plenty of memory left, however. It's only using 0.1% of the memory.
Does anyone have an idea what could cause this problem? Or suggestions about what to look for? Thanks.
(Sorry for the messy output!)
-
It would be good if you posted the smallest and simplest (compilable) program that demonstrates the problem.
A few things I am concerned with:
- Why are your member variables public?
- Why do you pass strings and vectors by value instead of by (const) reference?
- Does a Station object own its merchandise? If so, why have you not implemented or disabled the copy constructor and copy assignment operator despite declaring the destructor?
- Shouldn't your find member functions be declared const since they should not be modifying the observable state of the Station object?
-
Being a tad picky here but...
In addition to the above, that iterator should be a const_iterator and should be pre-incremented not post-incremented.
Don't write "this->" if you can avoid it. It's just extra clutter.
This "this->sector->galaxy->verbosity" breaks some rule I've forgotten the name of. There are too many levels of indirection there. An object certainly has no business knowing about the members of its members members, especially being that those are pointers!
Those tests should be delegated out to functions of the Sector or Galaxy objects.
Solve that design problem and you may very well also solve your bug.
