Using malloc and sizeof for a pointer to a C++ class

Question

Is the following ok?

int n=5;
map<string,int> * maps = (map<string,int> *)malloc(n*sizeof(map<string,int>));
for (int i=0; i<n; i++) {
    maps[i] = map<string,int>();
    char * i_str = (char *)malloc(10);
    sprintf(i_str,"%d",i);
    char * key = (char *)malloc(100);
    strcpy(key,"key");
    strcat(key,i_str);
    (maps[i])[string(key)] = i*i;
}

I know people say to use new rather than malloc in C++. But if I do it this way, what if any problems can occur?

Edit: Code compiles fine and runs fine (g++ 4.6.3). Not sure why the question is on hold, but I just want to know if it's technically correct (I don't care about style). One thing I'm not sure is whether the line

maps[i] = map<string,int>();

is syntactically correct, and whether it really does lead to unexpected behaviour as one person suggested.

Someone has yet to provide a reference to the C++ specification that shows that what I'm doing is undefined. So this question is still not answered.

It's terribly wrong. Use `std::vector>`. No `malloc`, no `new`, no excuses. — , Jul 17 '13 at 23:18
Why would you want to do this? C everywhere in C++ to initialize C++ containers? No. — Aleks, Jul 17 '13 at 23:20
I would like to know where you learned to do this so we can stop this at its source. — dreamlax, Jul 17 '13 at 23:27
I prefer to use C whenever possible, rather than C++. My question isn't about style, but whether the code technically works. — onelineproof, Jul 17 '13 at 23:42

score 3 · Answer 1 · edited May 23 '17 at 12:21

3

The main problem will be that the constructor of the map will not run.

Without the constructor, the map will be in an invalid state and will not work properly.

You can use placement new to manually call the constructor.

EDIT: Just realized that you do try to make the map valid with:

maps[i] = map<string,int>();

However, this will not work because the map needs to be in a valid state before it can be copied into (and it is currently not valid at that point because it has not been constructed yet).

There are also many memory leaks in your program as well. Make sure to either use smart pointers(note that you need to use new for smart pointers to work) or free every allocation.

edited May 23 '17 at 12:21

Community

1
1

answered Jul 17 '13 at 23:20

Mention the memory leaks and I'll give my +1 back – Mooing Duck Jul 17 '13 at 23:26
Ya I know about the memory leaks. I can just use free or delete for those, but that's not the point of the question. – onelineproof Jul 17 '13 at 23:54
@onelineproof For future reference, C++ has a category called POD for classes that do not need to be constructed. You can query if a class is a POD or not at compile time using [std::is_pod](http://en.cppreference.com/w/cpp/types/is_pod). Here is an example: http://ideone.com/gZ0sqH (Note that std::is_pod might not be implemented on old compilers). – Jul 17 '13 at 23:59

score 2 · Answer 2 · edited Jul 17 '13 at 23:29

2

The instruction:

map<string,int> * maps = (map<string,int> *)malloc(n*sizeof(map<string,int>));

reserves memory for storing maps, but does not initialize the memory (call the constructor) . Most likely the map's constructor here will require extra dynamic allocation.

Then

maps[i] = map<string,int>();

calls the map's copy assignment operator=, which supposed to clear the previous contents of the map, and replace it with new (blank) contents. However, since the former map is not in a valid state, you'll very likely go into trouble here. This is undefined behavior, and may do nothing, or it may crash, or do other random things, randomly.

You are also losing a lot of memory, since you never free the memory that you malloc.

edited Jul 17 '13 at 23:29

Mooing Duck

64,318
19
100
158

answered Jul 17 '13 at 23:23

hivert

10,579
3
31
56

@Mooing Duck: thanks for the edits – hivert Jul 17 '13 at 23:32
Ya its this line (maps[i] = map();) that I wasn't sure about. But it does compile and seems to give sane results. – onelineproof Jul 17 '13 at 23:36
1

Your program exhibits [undefined behavior](http://en.wikipedia.org/wiki/Undefined_behavior). "Seems to give sane results" is one possible manifestation of undefined behavior. – Igor Tandetnik Jul 17 '13 at 23:57

Using malloc and sizeof for a pointer to a C++ class

2 Answers2