0

I have a structure which has an std::list as its data member. That std::list is an collection of std::pair.

Like this(Inside .h file)

extern struct_info tt;
typedef struct_s *struct_info;
typedef 
struct struct_s {
  std::list < std::pair <char*, int> > names;
};

I am allocating memory for this structure in my .cpp file as:

tt = mem_malloc(sizeof(struct_t));

mem_malloc is my own memory allocating routine. Please note that it is already made extern in .h file.

Later, when I try to push_back any element into the list with following code:

std::pair <char*, int> myPair = std:make_pair("A", 5);
(tt->names).push_back(myPair);

It crashes while doing push_back. I do not know what is happening here. Do I need to call any constructor or initializer for the list in struct_s constructor?

What do you guys think?

Hemant Bhargava
  • 3,251
  • 4
  • 24
  • 45
  • 6
    Judging by how you are using `mem_malloc` I'm pretty certain it doesn't call the constructor of `struct_s` (and yes, you do need to call the constructor for non-trivial types) – UnholySheep Oct 22 '18 at 13:48
  • 1
    The simple solution is to call said constructor manually after allocating storage (`new (tt) struct_s;`) and call the destructor before deallocating (`tt->~struct_s;`). But IMO this kind of manual memory & lifetime management is a huge code smell. – HolyBlackCat Oct 22 '18 at 13:50
  • @UnholySheep, Actually I call it explicitly. Infact it is not constructor per se. I do something like this: tt = mem_malloc(sizeof(struct_t)) tt->first_member = FALSE (IF there is any) Btw, I do not do any thing here with the std::list. – Hemant Bhargava Oct 22 '18 at 13:50
  • 1
    That is not calling the constructor. The only way to call it would be to use the placement new syntax. – NathanOliver Oct 22 '18 at 13:51
  • @HolyBlackCat, Yes. That is right. It is huge code. – Hemant Bhargava Oct 22 '18 at 13:51
  • 1
    I meant `huge (code smell)`, not `(huge code) smell`. :/ – HolyBlackCat Oct 22 '18 at 13:52
  • @NathanOliver, Yes. Not exactly constructor. As I wrote, not constructor per se. But it is doing the things which constructor will do manually. – Hemant Bhargava Oct 22 '18 at 13:53
  • @HemantBhargava For arithmetic types - yes. For most classes - no. – HolyBlackCat Oct 22 '18 at 13:53
  • @HemantBhargava: if you don't call the constructor then there's no object and your code has undefined behavior. Also you have no idea what the constructor for `std::list` or `std::pair` is doing. – Vittorio Romeo Oct 22 '18 at 13:54
  • @VittorioRomeo, Please read my question. I have made an object and made that extern. That is what I am using in .c file. Allocating memory to it by doing my own mem_malloc. And then initialing things. – Hemant Bhargava Oct 22 '18 at 13:55
  • @HemantBhargava: if you don't use "placement `new`" after `mem_malloc`, you haven't made any object. – Vittorio Romeo Oct 22 '18 at 13:56
  • 1
    @HemantBhargava You don't make an object (in this case) until the constructor is called. If you don't call the constructor you don't have a valid object and instead you have undefined behavior. It looks like you are coming from C and expecting C++ to work like C does but it doesn't. They are different languages and have different lifetime and initialization rules. – NathanOliver Oct 22 '18 at 13:58
  • @VittorioRomeo, I am sure that mem_alloc is doing the right thing here. It is leagacy code for us. Having said that, If I do not include std::list in structure definition, all goes well. – Hemant Bhargava Oct 22 '18 at 13:58
  • If you don't believe us, see for yourself: http://coliru.stacked-crooked.com/a/7a9301051b112a96 – HolyBlackCat Oct 22 '18 at 14:00
  • @HemantBhargava: `mem_alloc` **cannot** be doing the right thing as the API doesn't support it. `mem_alloc` takes a size and not a type, which prevents it from being able to call the constructor. – Vittorio Romeo Oct 22 '18 at 14:01
  • 2
    @HemantBhargava You state `"I am sure that mem_alloc is doing the right thing here"`. Since `mem_malloc` has no notion of the type `struct_t` I'm pretty certain it's *not* doing the right thing. – G.M. Oct 22 '18 at 14:01
  • Possible duplicate of [Using malloc/realloc for array of classes/structs including std vector](https://stackoverflow.com/questions/38807403/using-malloc-realloc-for-array-of-classes-structs-including-std-vector) – Alan Birtles Oct 22 '18 at 14:02
  • @HolyBlackCat, Aah. Your example make things clearer. I am wrong it seems. I will have to call placement new operator here to resolve the things. Cool? – Hemant Bhargava Oct 22 '18 at 14:03

2 Answers2

5

You can't just allocate memory with size sizeof(struct_t) and expect to be able to use that memory as if a struct_t instance existed there - you need to construct it first. E.g.

tt = mem_malloc(sizeof(struct_t));
new (tt) struct_s{}; // <- "placement `new`"

// use `tt`...

tt->~struct_s(); // <- explicit destructor call
// deallocate tt

This is however a terrible idea, especially if done manually. The Standard Library provides allocator support - you should create an allocator that can be seamlessly used with the Standard Library instead.

Vittorio Romeo
  • 90,666
  • 33
  • 258
  • 416
-2

You can also do: struct_s obj; *tt = obj; . . . free(tt);

After experiments: 1. With non POD types my suggested solution will (must) not work. 2. For non POD types if above copy mechanism is applied, it will start accessing uninitialised memory location (in copy-to-location), assuming as if those are well-formed objects, and therefore will cause segmentation fault. 3. Point 2 makes it compulsory to initialize such malloced memory for non-POD types data properly. This in C is not possible and we will also not encounter such scenario there as there is everything is POD. This is the reason (I think, one of the reasons) C++ struct is made very similar to Class, so that, as suggested in other solutions, we can make appropriate initialization calls (i.e. constructors).

So the solution is to use placement new operator as suggested by others.

Vivek
  • 25
  • 3