find with char* variable doesnt work

Question

I'd like to know why I have a memory error with this:

The problem appears on char* value = aMap.find(keync)->second

If I put manualy char* value = "key0" it works!!!

using std::map;
map <char*, char*> aMap;

void search(const char* key) {
    const int LEN = strlen(key);

    char* keync = new char[LEN];

    for (int i= 0; i < LEN; i++) {
       keync[i] = key[i];
    }

    char* value = aMap.find(keync)->second;

    printf("%s", value);

    delete[] keync;
}

int _tmain(int argc, _TCHAR* argv[])
{
    a["key0"] = "value0";
    search("key0");

    return 0;
}

Answer 1

One obvious issue is the:

delete keync;

Because you used new [], it should be:

delete[] keync;

Answer 2

You need to add 1 to the length of the array:

char* keync = new char[LEN+1];

You're null terminating outside the string you allocated.

(Also, are you initialising aMap?)

Answer 3

You are better using std::string instead of char*. Another plus of using std::string is that you will avoid memory leaks.

The other solution will be to provide map with a comparator function, otherwise they will not compare the content of each char*, but instead the address pointed. The following example was adapted from Sgi's std::map documentation:

struct comp
{
  bool operator()(char* s1, char* s2) const
  {
    return strcmp(s1, s2) < 0;
  }
};

map<char*, char*, comp> stringMap;

Answer 4

Well look for starters you should drop all the char* stuff and use std::string, this would probably make your problem go away.

FYI:
keync[LEN] = '\0'; // this is wrong and will be one past the end of the allocated array

The copying of the key parameter is wrong. Try this on for size:

using std::map;
map <char*, char*> aMap;

void search(const char* key) {
    const int LEN = strlen(key);

    char* keync = const_cast<char*>(key);

    char* value = aMap.find(keync)->second;

    printf("%s", value);
}

int main(int argc, char** argv)
{
    aMap["key0"] = "value0";
    search("key0");

    return 0;
}

The problem you were having is because you were allocating keync as strlen(key) which doesn't count the null terminator. In your copy loop you were then overwriting the null terminator with the last char of key.

The whole idea of copying the input string is wrong and I have replaced it with a const cast in my solution as it makes more sense (as far as that goes).

Answer 5

As others pointed, you are much better off using std::string for this. Now, for the actual problem why you are not able to find the string is because you are storing pointers in the map i.e. the key to the map is a pointer variable. You inserted a char* in to the map but when you are trying to find you are doing a new again. This ia a totally different pointer (although the string value they point is same) hence your lookup will fail.

Answer 6

using std::map;
map <char*, char*> aMap;

first of all, this map won't compare strings (inside of search) but addresses. So basicly you won't find anything with std::map::search by typing string literals.

void search(const char* key) {
    const int LEN = strlen(key);

    char* keync = new char[LEN];

    for (int i= 0; i < LEN; i++) {
       keync[i] = key[i];
    }

at this moment you have unterminated string, but it doesn't matter in your code

    char* value = aMap.find(keync)->second;

here you're performing search by comparing pointers values (addresses), so returned map iterator is invalid (it's equal aMap.end()), so either it has null or unallocated pointer as second member

    printf("%s", value);

    delete[] keync;
}

int _tmain(int argc, _TCHAR* argv[])
{
    a["key0"] = "value0";
    search("key0");

    return 0;
}

I hope it explains you why should you use std::string instead of char *