malloc and new...

[dade]

Hi,

i use a dynamic 3D matrix of structure that yields to a 90Mb of RAM
allocated.
To do that i use new operator. here a portion of code

struct SInfo
{
int i1;
int i2;
float f1;
float f2;
}

SInfo ***pCoord = new SInfo **[iELM_MAT_R];

for(j=0; j<iELM_MAT_R; j++)
{
pCoord[j] = new SInfo *[iELM_MAT_C];
for(k=0; k<iELM_MAT_C; k++)
{
pCoord[j][k] = new SInfo[MAX_ELM];
}
}
and it works...but i read that it is better to use malloc and free
instead of new and delete to avoid memory leaks.

i don't see error in my code but i'd like to know if there is something
i'm skipping...

thanks

Davide

 

[VJ]

Hi,

i use a dynamic 3D matrix of structure that yields to a 90Mb of RAM
allocated.
To do that i use new operator. here a portion of code

struct SInfo
{
int i1;
int i2;
float f1;
float f2;
}

SInfo ***pCoord = new SInfo **[iELM_MAT_R];

for(j=0; j<iELM_MAT_R; j++)
{
pCoord[j] = new SInfo *[iELM_MAT_C];
for(k=0; k<iELM_MAT_C; k++)
{
pCoord[j][k] = new SInfo[MAX_ELM];
}
}
and it works...but i read that it is better to use malloc and free
instead of new and delete to avoid memory leaks.

i don't see error in my code but i'd like to know if there is something
i'm skipping...

thanks

Davide

It is bad to allocate many chunks of memory - better allocate one huge
chunk, and access it differently

SInfo *pCoord = new SInfo[ iELM_MAT_R * iELM_MAT_C * MAX_ELM );
or
std::auto_ptr< SInfo > pCoord;

you should probably make a class to use coordinates to calculate index
in this big structure



....or you can forget about this, and use vectors

 

[Kai-Uwe Bux]

Hi,

i use a dynamic 3D matrix of structure that yields to a 90Mb of RAM
allocated.
To do that i use new operator. here a portion of code

struct SInfo
{
int i1;
int i2;
float f1;
float f2;
}

SInfo ***pCoord = new SInfo **[iELM_MAT_R];

for(j=0; j<iELM_MAT_R; j++)
{
pCoord[j] = new SInfo *[iELM_MAT_C];
for(k=0; k<iELM_MAT_C; k++)
{
pCoord[j][k] = new SInfo[MAX_ELM];
}
}
and it works...but i read that it is better to use malloc and free
instead of new and delete to avoid memory leaks.

Huh? Where did you read that? The problems of memory management using
malloc/free are essentially the same as with using new/delete.

i don't see error in my code but i'd like to know if there is something
i'm skipping...

Well, your code leaks in the event that an allocation after the first is
unsuccessfull and throws a bad_alloc. You should consider using

vector< vector< vector< SInfo > > >

instead. That will take care of memory management for you. Be sure to use
reserve() for efficiency.


Best

Kai-Uwe Bux

 

[Daniel T.]

Hi,

i use a dynamic 3D matrix of structure that yields to a 90Mb of RAM
allocated.
To do that i use new operator. here a portion of code

struct SInfo
{
int i1;
int i2;
float f1;
float f2;
}

SInfo ***pCoord = new SInfo **[iELM_MAT_R];

for(j=0; j<iELM_MAT_R; j++)
{
pCoord[j] = new SInfo *[iELM_MAT_C];
for(k=0; k<iELM_MAT_C; k++)
{
pCoord[j][k] = new SInfo[MAX_ELM];
}
}

Better would be a class:

template < typename T >
class Array3D
{
public:
Array3D( unsigned x, unsigned y, unsigned z ):
l( x ), h( y ), w( z ), rep( x * y * z ) { }

T& at( unsigned x, unsigned y, unsigned z ) {
assert( x < l && y < h && z < w );
return rep[ x * l * h + y * h + z ];
}

const T& at( unsigned x, unsigned y, unsigned z ) const {
assert( x < l && y < h && z < w );
return rep[ x * l * h + y * h + z ];
}

private:
unsigned l, unsigned h, unsigned w
std::deque< T > rep;
};

(Someone double check me on the above, I didn't test it.)

and it works...but i read that it is better to use malloc and free
instead of new and delete to avoid memory leaks.

That's silly.

i don't see error in my code but i'd like to know if there is something
i'm skipping...

You should use const unsigned for your array bounds rather than defines,
you need to remember to delete all the memory you newed, and the code
you have above needs to be made exception safe.

Use a class like the one I have above instead. It is a complete solution
in fewer lines of code, easer to maintain and more likely to be correct.

 

[Kai-Uwe Bux]

Hi,

i use a dynamic 3D matrix of structure that yields to a 90Mb of RAM
allocated.
To do that i use new operator. here a portion of code

struct SInfo
{
int i1;
int i2;
float f1;
float f2;
}

SInfo ***pCoord = new SInfo **[iELM_MAT_R];

for(j=0; j<iELM_MAT_R; j++)
{
pCoord[j] = new SInfo *[iELM_MAT_C];
for(k=0; k<iELM_MAT_C; k++)
{
pCoord[j][k] = new SInfo[MAX_ELM];
}
}

Better would be a class:

template < typename T >
class Array3D
{
public:
Array3D( unsigned x, unsigned y, unsigned z ):
l( x ), h( y ), w( z ), rep( x * y * z ) { }

what about

typedef std::size_t size_type;

and then using size_type instead of unsigned throughout?

T& at( unsigned x, unsigned y, unsigned z ) {
assert( x < l && y < h && z < w );
return rep[ x * l * h + y * h + z ];

I get x*h*w + y*w + z

}

const T& at( unsigned x, unsigned y, unsigned z ) const {
assert( x < l && y < h && z < w );
return rep[ x * l * h + y * h + z ];
}

private:
unsigned l, unsigned h, unsigned w
std::deque< T > rep;

Is there a reason to use deque? You are not planning on changing the size
anyway, are you? This is a case, where std::vector with a call to reserve
might be best (or even, gasp, a T* with a new in the constructor and a
delete [] in the destructor.

};

(Someone double check me on the above, I didn't test it.)


That's silly.


You should use const unsigned for your array bounds rather than defines,
you need to remember to delete all the memory you newed, and the code
you have above needs to be made exception safe.

Use a class like the one I have above instead. It is a complete solution
in fewer lines of code, easer to maintain and more likely to be correct.

Best

Kai-Uwe Bux

 

[Frederick Gotham]

dade:

i use a dynamic 3D matrix of structure that yields to a 90Mb of RAM
allocated.

Your code does not exhibit a 3-dimensional array. The following is a 3-
dimensional array:

int arr[A][C];

, and is sits in memory exactly as if it were an int[A*B*C];

What you have is something like the following:

int i;

int *p;

int **pp;

p = &i;

pp = &p;

You have an array of pointers, and each pointer points to an array.

 

[Daniel T.]

what about

typedef std::size_t size_type;

and then using size_type instead of unsigned throughout?

Works for me. I felt doing so would confuse the OP even more.

T& at( unsigned x, unsigned y, unsigned z ) {
assert( x < l && y < h && z < w );
return rep[ x * l * h + y * h + z ];

I get x*h*w + y*w + z

Yea, that's what I was thinking I got wrong. Thanks.

}

const T& at( unsigned x, unsigned y, unsigned z ) const {
assert( x < l && y < h && z < w );
return rep[ x * l * h + y * h + z ];
}

private:
unsigned l, unsigned h, unsigned w
std::deque< T > rep;

Is there a reason to use deque?

Less likely to fail if the memory is fragmented. The OP is planaing on
making a 90MB block after all.

You are not planning on changing the size anyway, are you?

The OP may be planing on changing the size, I wouldn't know.

This is a case, where std::vector with a call to reserve
might be best (or even, gasp, a T* with a new in the constructor and a
delete [] in the destructor.

As written a call to reserve is meaningless since the container would be
created with all elements.

The nice thing about this class for the OP is that he has the option of
replacing the container type without the rest of the code caring. For
example, it may be the case that he doesn't really need to allocate the
whole 90MB at construction, this class allows for a lazy initialization
of cells.