-3

I need to free (to avoid memory leaks) the all allocated memory for following C++ code(std::map, both key & value created by using new).

int main()
{
    std::map<char*, char*> *mp = new std::map<char*, char*>;
    char *a = new char;
    a = (char*)"abc";
    char *b = new char;
    b = (char*)"pqr";
    mp->insert(std::pair<char*, char*>(a, b));
    a = NULL , b = NULL; // no extra pointers to keys now //

    printf("element : %s", (*mp)["abc"]); // working

    // need to free the allocated memory by the map in here properly,
    // clear() & erase() are ok? Because I think, I need some 'delete's
}
Swordfish
  • 12,971
  • 3
  • 21
  • 43
yasara malshan
  • 370
  • 3
  • 11
  • 5
    *both key & value created by using new* – Why? Use `std::string`s. – Swordfish May 19 '19 at 04:29
  • 2
    The assignments to `a` and `b` after the definition and initialization both leak memory. – Jonathan Leffler May 19 '19 at 04:32
  • 2
    *`char *a = new char; a = (char*)"abc";`* – Here you are leaking memory by first allocating a `char` and then overwriting the address of that allocated memory with the address of a string literal (`"abc"`). If you hope to copy `"abc"` to the memory allocated with `new` before using the assignment operator (`=`) you are wrong. You're only assigning pointers not copying the data pointed to. To copy c-strings use `std::strcpy()`. Also, `new char;` only allocates memory for *one* character but the string `"abc"` is 4 characters long. 3 characters + 1 terminating `'\0'` character. – Swordfish May 19 '19 at 04:33
  • 3
    `std::map mp; mp.insert({"abc", "pqr"});` -- All of that code you wrote just to do that one single line? There is no memory leakage to worry about in that one line. – PaulMcKenzie May 19 '19 at 04:35
  • thanks @Swordfish & @PaulMcKenzie; but the thing that i really need to know is , how to free a map whose keys are created by 'new' or malloc (explicitly allocated). i can't do something like "delete mapIter->first" right..? – yasara malshan May 19 '19 at 04:40
  • 1
    *i can't do something like "delete mapIter->first"* -- Why not? You `delete` them just like any other pointer allocated. Just because those pointers are in a map doesn't mean they're not available to you. The map knows nothing about where those pointer values came from, so it's up to you to get to those pointers and call `delete`. – PaulMcKenzie May 19 '19 at 04:45
  • 6
    It's troubling that you're using `new` at all for a task like this. Are you using `new` because you believe it to be the normal way to write code like this? I worry that you're asking for help to develop bad habits. – Drew Dormann May 19 '19 at 04:49
  • ^^^ Sound advice from @DrewDormann. – PaulMcKenzie May 19 '19 at 04:52
  • thanks PaulMcKenzie . Drew Dormann, i know this is a bad habit in coding , but i nedd to know the way for de-allocate for given code(because i saw something like that in old codes, thats the reason). – yasara malshan May 19 '19 at 04:57
  • 2
    @yasaramalshan If you see that in "old codes" it's time to start planning a refactor. But you need to learn the language, and the most common parts of the standard library *first*. There are many [good text](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list?r=SearchResults&s=1|527.1924) on the subject, as well as [the best online language+lib reference you'll find](https://en.cppreference.com/w/) – WhozCraig May 19 '19 at 05:00
  • 1
    Dynamically allocated map keys is rarely a good idea. – molbdnilo May 19 '19 at 06:48

1 Answers1

2

The correct and safe way to write the same piece of code is to use std::string. This way, memory allocation is done behind the scene and you don't have to free it yourself. As proposed by @PaulMcKenzie, use

std::map<std::string, std::string> mp;
mp.insert({"abc", "pqr"});

However, I understand that in production some piece of code are not that easy to refactor and that you may have to work with what you have to avoid inserting more bugs. Here is the commented code that frees the allocated memory.

int main()
{
    std::map<char*, char*> *mp = new std::map<char*, char*>;
    //char *a = new char; // Do not allocate memory here since 'a' will point to a
    a = (char*)"abc";     // string literal and loose track of it's allocated memory
    //char *b = new char
    b = (char*)"pqr";
    mp->insert(std::pair<char*, char*>(a, b));
    a = NULL , b = NULL; // no extra pointers to keys now //

    printf("element : %s", (*mp)["abc"]);

    delete mp ;

    // You don't need to delete the keys/values because we
    // avoided allocating memory for it in the first place
}

Moreover, you need to be careful when working with char * as key value of std::map (see here) since without comparator, it compares the pointers, not the strings.