STL MAP Question

Looking at the code that I slapped together, does "clear()" from the map function leave a hole in memory? If so, how should I do I fix this?

Code:

---


#include <stdlib.h>
#include <map>

struct FOO {
  char *A;
  char *B;
  char *C;
  FOO &set_info( char *ptrA, char *ptrB, char *ptrC ) {
    A = ptrA;
    B = ptrB;
    C = ptrC;
    return *this;
  }
};

typedef map<char *, FOO> MAP_TD_FOO;


int main ( void ) {
    MAP_TD_FOO msi;
    MAP_TD_FOO::iterator itor;
    FOO foo;

    char *a, *b, *c;

    a = new char;
    b = new char;
    c = new char;

    a = (char *)"Just";
    b = (char *)"another";
    c = (char *)"test";

    msi[ a ] = foo.set_info( a, b, c );

    char *key;
    itor = msi.begin();

    while( itor != msi.end() ){
      key = (*itor).first;
      foo = (*itor).second;
      printf( "%s %s %s\n", foo.A, foo.B, foo.C );
      itor++;
    }

    msi.clear();  //  <------------Does clear leave a hole in memory?

    exit ( 1 );
}

---

Thanks to any and all that respond.

DeadPoet

I believe alot of questions regarding STL things can be answered here. To answer your question more specificly, I think it would be stupid if they built in a memory problem in a standard function (or what you want to call it) so I would surely bet on no.

Well, here is a follow-on to the scenario. If one tries to perform the following operation it causes a "Attempt to free unallocated object at <memory_address>".

Code:

---

    itor = msi.end();
    while ( itor != msi.begin() ){
      key = (*itor).first;
      foo = (*itor).second;
      delete foo.A;
      delete foo.B;
      delete foo.C;
      delete key;
      itor--;
    }

---

So, if I try a delete [] foo.x, as in the next example, I also get a "Attempt to free unallocated object at <memory_address>":

Code:

---

    itor = msi.end();
    while ( itor != msi.begin() ){
      key = (*itor).first;
      foo = (*itor).second;
      delete [] foo.A;
      delete [] foo.B;
      delete [] foo.C;
      delete [] key;
      itor--;
    }

---

So with that said, how is this "newed" memory being released? The STL for map states that clear does the following:

void clear();

The clear() function deletes all elements from the map.

But it has been my experience that if you stick a pointer into some container and if that item was created using new or malloc, then the item in question must be released back by using delete or free.

I understand that clear() function deletes all the elements from the map but does it release the memory from items that were created with new or malloc?

DeadPoet

STL containers relieve you of the need to allocate/deallocate memory used by the container. It does it for you. When you pass a data to a map using an insert() function or the [] operator does an insertion, the insert() function or the [] operator allocates memory without your direct involvement. Likewise, when you call an erase() function or the clear() function, STL deallocates memory without your direct involvement. If you don't call new to get the data added to the map, you shouldn't call delete to remove it either. Only call delete on memory you allocate with new.

In your original post it would be after the following line:
msi[ a ] = foo.set_info( a, b, c );

delete a;
delete b;
delete c;

The reason is that you don't know for sure how STL does the allocation for memory associated with the map, and it doesn't really matter. However it's done, it's not using the same memory you have already reserved for a, b, and c. So you need to delete the memory you allocated for a, b, and c and you need to let STL delete the memory it allocates for each of he nodes in the map.

BTW you can't assign one Cstyle string (string using a char *) to another using the assignment operator, you need to use strcpy() or a function similar to strcpy(). You can assign an STL string to another STL string, which is one of the advantages of using an STL string.

Likewise, I hope the board ate some square brackets. Otherwise, you try to place a group of characters into memory you have allocated for just a single char when you do this:

a = new char;

a = (char*) "test";

Thank you for your response, that is a great explaination of what is going on inside the STL.

DeadPoet

Just a few more question on this matter.

If a, b, and c are newed pointers to a character array then by deleting the pointer (like delete [] a) right after the map insert (msi[ a ] = foo.set_info( a, b, c );) then, you have destroyed the memory location that points to either a, b, and c thus, the map will not have access to the information. Is this correct or am I not seeing the bigger picture because this is what I am experiencing?

Questions Raised
1. How do I get around this scenario?
2. Can I just iterate though the map and delete the character array from both the key and value.
3. I if do so, above, will this confuse the map container?

DeadPoet


I think I may have answered my own questions. Yes to all, just if the key is also an elemet in the map just delete the pointer to only one of them. C++ does not like it if you try to delete a pointer to the same thing twice, IMAGINE THAT. :o

If anyone has any suggestions, I would be glade to hear them.

Thanks,

DeadPoet :D

I will echo elad's suggestion to use the STL strings. In this instance, it will make life much easier, because you will never have to use new or delete with them. You don't have to worry about memory, because everything is created on the stack and will automatically be removed.

Now, if you do want to store pointers to dynamically allocated memory in a map, you will just have to pay attention to who has ownership of that memory. What I mean is, if you use new (or new[]) to allocate memory, then you usually save a pointer to that memory. Eventually you call delete (or delete[]) with that pointer. If you make a copy of that pointer (like your original example does twice - once in FOO::setinfo() and once when inserting into the map), then you as a coder must decide who will eventually delete the allocated memory. As you have found out, the map does not automatically delete the memory for you, it just removes the entry in the map.

Of course, that memory can be deleted in one and only one place. Your choices for who has ownership of the pointer (and therefore will delete the memory at the right time) include the map, the FOO struct, and the main app itself. This makes sense because there are three copies of the pointer lying around.

1. Main App: Leaving ownership within the main function is good because it is where the memory was originally allocated. You don't have to transfer ownership, and so the other pieces of code (the map and the FOO struct) can assume they don't have to worry about the memory allocation or deallocation. In this case, you would have to make sure the map no longer uses its copy of the pointer - either by calling clear() to clear the map, or by removing the specific entry in the map whose pointer you are about to use with delete []. Also, make sure there are no other copies around before deleting it. The FOO structure will be removed when the map entry is removed, so it should be fine as long as the map is cleared.
2. FOO struct: Adding a destructor to the FOO struct is one way to go. That destructor could delete the pointers if they are not null. You would have to add a constructor to make sure the member variables start out as 0 until setinfo() is called. The downside of this is that you are transferring ownership, and so you must be sure not to use the original pointers if the FOO struct goes out of scope and its destructor is called. In this example, the data would be deleted automatically when the map is cleared, because the FOO destructor will be called automatically. This is not safe, however, because your class does not have a copy constructor and operator=, you have to guard against changing the value in the map to something different than the key, and other assorted scary things that could cause crashes.
3. The map: If you transfer ownership to the map, then you would have to write extra code that deletes the pointers inside each FOO struct before removing the entry from the map. So if you want to call clear, or if you are just done with the map and it is going out of scope, you must write the loop to go through each entry. Otherwise, you will get a memory leak.

I think the first option is the best one in this case, although with a little extra work it is quite common to use option 3 and let the map keep ownership of the pointer. The key is to remember where all the copies of the pointer are and make sure one and only one copy will delete the allocated memory. Of course. all of this is true regardless of whether you are using the STL map or not. In this case the map is just like any other structure that doesn't know what it holds.

Aside from any map issues, this code not only leaves a hole in the memory, but also opens a wide area for undefined behavior:

Code:

---

    a = new char;
    b = new char;
    c = new char;

    a = (char *)"Just";
    b = (char *)"another";
    c = (char *)"test";

---

CornedBee,

So far, you have not steered me in the wrong direction. So I ask you to please explain the "undefined behavior", just a little. You have sparked my interest on the matter.

If a, b, and c are pointers then why can, more so should, I not do this.

Code:

---

    a = new char;
    b = new char;
    c = new char;

    a = (char *)"Just";
    b = (char *)"another";
    c = (char *)"test";

---

The way that I see it is that a is now pointing to another pointer. Is this not correct?


DeadPoet

The first three lines dynamically allocate three characters and point each of the pointers to one of them.

In the second, you reassign the pointer addresses to literal strings. This creates two problems. First, the addresses of the three dynamic characters are now forgotten. You cannot recover them, which means that you cannot delete the memory, which means that you have a memory leak.
Second, literal strings are by the C standard defined to be of type const char *, because they are supposed to be immutable. You forcibly cast them to non-const char * (you should never use casts carelessly!) which opens the door for their modification, which is undefined behavior: the C standard doesn't say what happens if you do it, so anything might happen. The most common cases would be access violations or modifications of strings somewhere else in your app.

If you actually want to point to mutable strings, you must use array new and strcpy:

Code:

---

char *a = new char[strlen("Some text") + 1];
strcpy(a, "Some text");

---

This also means that you later need to delete[] the pointer.

It is far easier to use the standard string class, where you can simple write
std::string a("Some text");

Thanks for the quick reply on this matter. That is a great explaination of what I should not be doing. I will have to check my souce to verify that I have not done anything like that, I expect that I might have and thus I will need to take corrective action.

Once again you have been a solid source of great information.

Sincerely,

DeadPoet

You're welcome, as always.