Failed to allocate an array of pointers to a struct

Question

I'm trying to allocate an array of pointers to a struct but there's something wrong with my code.

This is my struct:

struct Brick {
  GameObject2D* handle_;
};

Brick** bricks_;

And this is how i'm trying to allocate memory for it:

int bricks_amount_ = 10;    

bricks_ = (Brick**)malloc(sizeof(Brick*) * bricks_amount_);

The program crash. I've make a devenv on it to debug where's the problem and it crash on this line:

for (unsigned short int i = 0; i < bricks_amount_; i++){
  bricks_[i]->handle_ = new GameObject2D(); <---------- CRASH!
}

Any suggestions?

PS: Sorry for my english :P

=========================================================================

[SOLUTION]

Finally i've decided to use std::vector instead raw pointers:

bricks_.resize(bricks_amount_);

but i've tried to make the malloc different way and it works too:

bricks_ = (struct Brick*)malloc(sizeof(struct Brick) * bricks_amount_);

or this:

bricks_ = new Brick[bricks_amount_];

Thank you to the people who wants to help!

Clearly `bricks_[i]->handle_` accesses uninitialized memory. `bricks_[i]` hasn't been assigned a value. — Kerrek SB, May 22 '16 at 18:46
Also don't mix `malloc()` and `new()`, use `new []` for allocating arrays. — πάντα ῥεῖ, May 22 '16 at 18:48
Additional explanation to @KerrekSB' comment (+1): malloc creates a usable array, but the contents are uninitialized, meaning that your Brick* pointers are all at a completly random value, possibly null, possibly pointing to beyond the stars... What you missed was `game_state_.bricks_[i] = new Brick();`, e. g. right before your crash, to initialize each pointer within the array. — Aconcagua, May 22 '16 at 18:57
@M.M But even with e. g. ::std::unique_ptr, you still have to assign a newly created Brick instance to it - lack of which caused the actual error... — Aconcagua, May 23 '16 at 05:47
@M.M `vector`, then. Critical due to possible reallocations, however, if needing pointers (raw or not) to the bricks from elsewhere. Pity we do not know TO's requirements... — Aconcagua, May 23 '16 at 08:28
I'm gonna try to use std::vector, because for objects i have no problem, but with struct yes... — tonimarquez84, May 23 '16 at 08:42
@tonimarquez84 What is the difference between an object and a struct..? — underscore_d, Jun 06 '16 at 12:04

score 0 · Accepted Answer · answered May 22 '16 at 18:47

0

It's C++:

don't use malloc, use new
don't use plain arrays, use std::vector or std::array
don't use raw pointers, use std::unique_ptr

answered May 22 '16 at 18:47

Jack

131,802
30
241
343

1

Something like this: bricks_ = new Brick [bricks_amount_]; – tonimarquez84 May 23 '16 at 08:38

score -1 · Answer 2 · answered May 22 '16 at 18:57

-1

Do not use

Bricks **   //will be a 2 dimensional array

Use instead

Bricks *   //will be a 1 dimensioanal array

If you want a single dimensional array

Then do

`Bricks * bricks_  = (Bricks *)malloc(sizeof(Brick) * bricks_amount_ );

`

Then you can safely do without a crash

for (unsigned short int i = 0; i < bricks_amount_; i++){


bricks_[i]->handle_ = new GameObject2D(); <----------   There will not be CRASH!
}

answered May 22 '16 at 18:57

ATul Singh

490
3
15

1

This is not correct - using malloc, you still get an unitialized array (that needs to be initialized via placement constructor!). Additionally, this way, you need to use `bricks[i].handle_` then. – Aconcagua May 22 '16 at 19:02
Yup @Aconcagua you are correct, These are still unitialized pointers, but if the struct is containing only 1 pointer and these will be alloacted with new in the very next step , then its fine. – ATul Singh May 22 '16 at 19:15
1

Still you circumvented the contstructor (remember, this is C++), and if ever the Brick class changes (more precise: receives non POD members), you might get into trouble (OK, you still can call the appropriate placement constructors for each member of type ::std::string, ::std::vector or whatever else class ::myNamespace::MyClass is used - and change your loop every time the Brick class changes). – Aconcagua May 22 '16 at 19:26
1

It just looks like one - but the Bricks actually are never created because of using malloc instead of new Brick[n], which would call the constructor. – Aconcagua Jun 01 '16 at 00:28
@Aconcagua , yes you are right but his Bricks object contains only a pointer which will be set in the next set of instructions. This solution will only work when Bricks objects remains same as the way it is right now. – ATul Singh Jun 05 '16 at 07:12
1

@ATulSingh You pushed me towards an interesting [question](http://stackoverflow.com/q/37644977/1312382) concerning undefined behaviour. Apparently, in the current case, it is OK, but as soon as a constructor is added to the class, we receive UB... – Aconcagua Jun 06 '16 at 18:11
1

@ATulSingh Have to correct myself - the mentioned thread evolves more and more, and it seems to be undefined behaviour even for POD objects. – Aconcagua Jun 08 '16 at 20:24

Failed to allocate an array of pointers to a struct

2 Answers2

Linked