views:

123

answers:

3

I have a function which creates an array of Maps:

map<string, int> *pMap

And a function which writes maps to the array:

int iAddMap(map<string, int> mapToAdd, map<string, int> *m, int i)
{
    m = &(pMap[i]);
    memcpy(m, mapToAdd, sizeof(map<string, int>));
}

And a function to get maps from the array

map<string, int>& getMap(int i)
{
    return pMap[i];
}

I can write maps to the array without any issue, but every get call results in a seg fault:

int val; 
// val defined after this
map<string, int> * pGetMap = &(getMap(val));

Any suggestions on why this is happening?

+8  A: 

You cannot use memcpy() to copy objects like maps, or indeed any other kind of C++ container - you need to use assignment, which takes into account the map's underlying structure and se,mantics. And you should be using a vector <map>, as you actually seem to want a copy rather than a pointer.

anon
+3  A: 

This seems like an unholy mix of C and C++ programming, begin with changing:

memcpy(m, mapToAdd, sizeof(map<string, int>));

to

*m = *mapToAdd;

Also in the function

int iAddMap(map<string, int> mapToAdd, map<string, int> *m, int i)

What is the purpose of m? Any modifications to m won't be seen outside of this function.

Andreas Brinck
You probably meant *m = *mapToAdd;
Yossarian
@Yossarian Thanks
Andreas Brinck
knittl
@knittl What vector? Both `m` and `mapToAdd` are pointers to `std::map` so doing `*m = *mapToAdd` will assign the map `mapToAdd` is pointing at to the map `m` is pointing at.
Andreas Brinck
+4  A: 

Never ever use memcpy for copying an object of a map (or whatever class, for that matter) - use assignment or copy constructor instead. memcpy is a C function, which has no idea of the internal structure of a C++ object, so it is almost guaranteed to mess things up.

Also you would better use an std::vector<map<string, int> > instead of an array:

std::vector<map<string, int> > maps;

void addMap(const map<string, int>& mapToAdd, int i)
{
    maps[i] = mapToAdd;
}

map<string, int>& getMap(int i)
{
    return maps[i];
}

Note: addMap does not return anything in your example code, so I changed its return type to void. Also I pass mapToAdd as const reference rather than by value.

Péter Török