problems understanding std::map

[Antti Granqvist]

Hello!

I have following object relations:

Competition 1--* Category 1--* Course
1
|
|
*
Course

In competetition.h:
class Competition
{
public:

// type definitions
typedef std::string name_type;
typedef long date_type;
typedef long time_type;
typedef short storage_type;
typedef std::map<name_type, Course> Courses;
typedef std::map<name_type, Category> Categories;
...
// Constant definitions
static const storage_type BINARY_FILE = 1;
static const storage_type ASCII_FILE = 2;
static const storage_type DATABASE = 3;

...
void loadCourses(const name_type filename = "radat.lst",
const storage_type storage = ASCII_FILE);
...
private:
struct CompetitionImpl * m_CompRep;
...
}

I am trying to populate a Competition's Courses by reading a text
file.
The lodadCourses implementation in competition.C:

void CompetitionImpl::loadCoursesFromAscii(const name_type fileName) {

const char * fname = fileName.data();

// is usage of automatic variable correct here???
name_type name;
Course course;
Course::code_type code;
std::string line,codes;

std::ifstream infile(fname, std::ios::in);
PRE_(infile, std::ios_base::failure);
int state = 0;
std::string::size_type ind;
while(getline(infile, line)) {
switch (state) {
case 0:
name = line.substr(0, line.find(" "));
state = 1;
break;
case 1:
ind = 0;
course = Course(name);
for(codes = nextToken(line, ' ', &ind);
codes != "" && codes != "0";
codes = nextToken(line, ' ', &ind)) {
code = (short)atoi(codes.data());
course.addCheckPoint(code);
}


state = 2;
break;
case 2:
ind = 0;
for(codes = nextToken(line, ' ', &ind);
codes != "" && codes != "0";
codes = nextToken(line, ' ', &ind)) {
code = (short)atoi(codes.data());
course.addCheckPoint(code);
}
m_Courses[name] = course; // segfault from map here!
state = 0;
break;
default:
break;
}
//std::cout << line << "\nstate:" << state << std::endl;
}
//ifstream destructor takes care of closing file!

}


course.h:

class Course
{
public:

// type definitions
typedef std::string name_type;
typedef short code_type;
typedef std::vector<code_type> codes;
Course();

Course (name_type&);

Course& operator=(const Course&);


~Course();

bool operator==(const Course&) const;

name_type getName() const;
code_type getFirstCheckPoint() const;
code_type getCheckPoint() const;
void addCheckPoint(const code_type p_Code);

// test validity of object:
bool Invariant () const; // for debugging

private:
struct CourseImpl * m_CourseImpl;



};

course.C:
// Internal representation of Course class.

struct CourseImpl {

// typedefs
typedef Course::name_type name_type;
typedef Course::code_type code_type;
typedef Course::codes codes;

// members
name_type m_Name;
codes m_Codes;

// to test the validity of internal state:
bool Invariant() const; // express the valid state
void Post () const; // check the class invariant

CourseImpl(name_type&, codes&);
CourseImpl(codes&);

private:
CourseImpl (const CourseImpl&);
void operator = (const CourseImpl&);

};

// #define NDEBUG // default: debug is on
// while debugging, ignore exceptions and use assert
// Note. If NDEBUG is defined, all asserts are ignored.
//
// precondition and postcondition tests:
#ifndef NDEBUG // debug is on: test everything
#define PRE_(pre,E) assert (pre)
#define POST_() assert (this->Invariant ())
#else // for production code, only obligatory tests
#define PRE_(pre,Excpt) if(!(pre)) throw (Excpt)
#define POST_() /* empty */
#endif

// check correctness of internal state
inline bool CourseImpl::Invariant () const
{
if (this == 0) return false;
return true;
} // Invariant

// test the class invariant:
inline void CourseImpl:ost () const
{
POST_();
}

inline CourseImpl::CourseImpl(name_type& p_Name, codes& p_Codes):
m_Name(p_Name), m_Codes(p_Codes){

}

inline CourseImpl::CourseImpl(codes& p_Codes): m_Codes(p_Codes) {
}



/////////////////////////////////////////////
//
// Implementation of Course public interface
//
/////////////////////////////////////////////

Course::Course(Course::name_type& p_Name) {
codes c = codes(0);
m_CourseImpl = new CourseImpl(p_Name, c);
}

//Default constructor needed by map!

Course::Course() {
codes c = codes(0);
m_CourseImpl = new CourseImpl(c);
}

// Test state of Course instance
bool Course::Invariant () const {
return m_CourseImpl!= 0 && m_CourseImpl->Invariant ();
}
Course& Course:perator=(const Course& rhs) {
POST_();
if(&rhs != this) {
m_CourseImpl->m_Codes = rhs.m_CourseImpl->m_Codes;
m_CourseImpl->m_Name = rhs.m_CourseImpl->m_Name;
}
return *this;
}

Course::~Course() {
delete m_CourseImpl;

}


bool Course:perator==(const Course& rhs) const {
POST_();
return m_CourseImpl->m_Codes == rhs.m_CourseImpl->m_Codes;
}

Course::name_type Course::getName() const {
return m_CourseImpl->m_Name;
}

Course::code_type Course::getFirstCheckPoint() const {

//PRE_(m_CourseImpl->m_Codes.size() > 0, std:ut_of_range);
if (m_CourseImpl->m_Codes.size() > 0)
return m_CourseImpl->m_Codes[0];
else
return short(0);
}

void Course::addCheckPoint(Course::code_type p_Code) {
m_CourseImpl->m_Codes.push_back(p_Code);
}

When running the main program:
int main() {

Competition comp;
comp.loadCourses();
...

I get a segfault and from gdb I can get following backtrace:
0x00428ce1 in std::string::~string() ()
at /usr/include/c++/3.3.1/bits/stl_algobase.h:402
402 copy(const _Tp* __first, const _Tp* __last, _Tp*
__result)
(gdb)
(gdb) bt
#0 0x00428ce1 in std::string::~string() ()
at /usr/include/c++/3.3.1/bits/stl_algobase.h:402
#1 0x00412851 in CourseImpl::~CourseImpl() (this=0xa047010) at
course.C:99
#2 0x00405b8f in Course::~Course() (this=0x22ea80) at course.C:99
#3 0x004316be in std::map<std::string, Course,
std::less<std::string>, std::allocator<std:air<std::string const,
Course> > >:perator[](std::string const&)
(this=0xa043b80, __k=@0x22ed00)
at /usr/include/c++/3.3.1/bits/stl_map.h:319
#4 0x0040254b in CompetitionImpl::loadCoursesFromAscii(std::string) (
this=0xa043b70, fileName=0x22ed90) at competition.C:153
#5 0x00403738 in Competition::loadCourses(std::string, short)
(this=0x22eef8,
p_Name=0x22eed8, p_Type=2) at competition.C:317
#6 0x0040122c in main () at main.C:8

Run on cygwin and compiled with g++ 3.3.1 (cygming special)

I suspect using automatic variables is not the right way to set
objects into a std:map, or is the problem in Course class and its
implementation? Can anyone with more experience using C++/stl explain
what is the right way of doing this?
Any help and references to further reading appreciated.

 

[David Harmon]

m_Courses[name] = course; // segfault from map here!

I suspect using automatic variables is not the right way to set
objects into a std:map, or is the problem in Course class and its
implementation?

Yes, using automatic variables is perfectly ok. Of course std::map will
make its own copies of them to store, so it is crucial that the copy
constructor and assignment operator of class Course works properly.
In that regard, the line

struct CourseImpl * m_CourseImpl;

worries me. Using such a risque technique means you must watch your
allocated memory very carefully indeed in order to avoid memory leaks
and/or multiple copies trying to share the same m_CourseImpl.
Where is your copy constructor?

That input parsing logic with all the state switching is mind boggling.
Quite likely it's wrong and what you are trying to insert in the map is
not what you think. As a trace I'd add

cerr << "Inserting m_Courses[" << name << "] =" << course << '\n';

with suitable operator<< defined for Course.

Also possible that a stray write elsewhere trashed the map earlier, etc.