0

I have a struct:

struct Foo
{
    std::unordered_map<std::string, std::string> info;
    int count;
    int bar;
}

I am trying to initialize this struct on the heap as follows:

Foo* createFoo(int count, int bar)
{
    Foo* foo = (Foo*)malloc(sizeof(Foo));
    foo->info = std::unordered_map<std::string, std::string>(); // <- exception thrown here
    foo->count = count;
    foo->bar = bar;
    return foo;
}

I am getting the following exception thrown upon construction of the unordered_map:

Exception thrown: read access violation. _Pnext was 0xCDCDCDD1.

I understand that MVS fills heap allocated memory with 0xCD which is why the _Pnext pointer has this value, but I don't understand why the unordered_map constructor isn't zero-ing these fields out.

I realize that the modern C++ way of doing this is with new/constructors but I am trying to write this code in a non-OOP procedural way with (basically) POD objects.

Am I initializing the map incorrectly?

Olivia Stork
  • 4,660
  • 5
  • 27
  • 40
chips
  • 160
  • 1
  • 10
  • 1
    A `unordered_map` is by no means a POD-type. Why do you try it this way when you know how it _should_ be done? – Lukas-T Aug 24 '20 at 11:52
  • correct me if I'm wrong but C++ is not an OOP language but rather allows OOP features - are you saying that there is no way of initializing an unordered_map in place like this? – chips Aug 24 '20 at 11:57
  • 1
    Using `new` is not the "modern" way; it is and has always been *the* way, ever since C++ was called "C with classes". It also has absolutely nothing to do with OOP. – molbdnilo Aug 24 '20 at 11:58
  • 1
    @chips C++ is multiparadigm language, OOP is one of the paradigms it enables. And it's not about initializing `std::unordered_map`, you have a problem with initializing `Foo`. Initialization of `Foo` will initialize all of its members. – Yksisarvinen Aug 24 '20 at 12:05
  • 2
    May I recommend a good [C++ Book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). Will save a ton of time. – rustyx Aug 24 '20 at 12:09
  • 3
    _Am I initializing the map incorrectly?_ You are not initializing the map at all. – Eljay Aug 24 '20 at 12:19

3 Answers3

1

A C call of malloc

Foo* foo = (Foo*)malloc(sizeof(Foo));

does not invoke constructors for data members.

So the data member

std::unordered_map<std::string, std::string> info;

was not constructed.

And this statement with the copy assignment operator

foo->info = std::unordered_map<std::string, std::string>();

results in undefined behavior because there is not created the object foo->info.

You have to use the operator new instead of malloc.

For example

Foo* foo = new Foo();
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
1

malloc() do not initialize allocated memory and this is bad when allocating objects that have non-trivial constructors.

You should use new instead.

Foo* foo = new Foo;

To deallocate objects allocated via new, you can use delete.

delete pointe_to_object;
MikeCAT
  • 73,922
  • 11
  • 45
  • 70
1

malloc() doesn't create any object. It just reserves some uninitialized memory. There are no objects in that memory, those would have to be created with placement new.

Now, operator= requires an existing object, because it's a member function (always). By calling this operator on line foo->info = std::unordered_map<std::string, std::string>() you call operator on non-existing object.

The solution is to not go against the language and use new as you are supposed to:

Foo* createFoo(int count, int bar)
{
    Foo* foo = new Foo;
    // unnecessary now, the object is already constructed and default-initialized
    // foo->info = std::unordered_map<std::string, std::string>(); 

    // ints are constructed, but not initialized
    foo->count = count;
    foo->bar = bar;
    return foo;
}

You could also use malloc with placement new, but that's only useful if you need memory without actual objects (e.g. in vector implementation).


Note: it's a bad smell to use raw new in modern C++ (well, 9 years old, but as modern as std::unordered_map). Use smart pointers and STL containers instead.

Yksisarvinen
  • 18,008
  • 2
  • 24
  • 52
  • as far as I was aware all new does is allocate the memory and then call the default constructor (is this not correct?) if it does then I still don't see why the default constructor can't be invoked manually in place.... – chips Aug 24 '20 at 12:08
  • 1
    @chips It's incorrect. You are describing behaviour of `new`, `malloc` only allocates memory, nothing more. And you can invoke constructor manually, using placement `new`, as explained in comment under some other answer. But you are trying to invoke `operator=`, not just default constructor, this is UB. – Yksisarvinen Aug 24 '20 at 12:12
  • I understand what malloc does. I've just noticed the placement new comment which answers my question. If operator= is UB not sure why my compiler isn't calling me all sorts of names. – chips Aug 24 '20 at 12:18
  • UB means "anything can happen". A crash (like when using not initialized non-POD structure) or seemingly working correct (like when using not initialized `int`) are both perfectly fine examples of UB. If compiler was obliged to point out UB, it would not be UB anymore. – Yksisarvinen Aug 24 '20 at 12:22