Question about destructors

I have been working with C++ for about four years now, mostly self-
taught, and I get along reasonably well. However, last week I detected
a memory leak in some code, and trying to locate it I realized that I
do not know enough about destructors. I have created a number of use
cases. How can I make sure in each case that the memory gets destroyed
properly?

1. int *x[10];

2. int *y = new int[10];

3. Object *obj = new Object();

4. Object **obj = new Object[n];
for (int i=0; i<n; i++) obj[i] = new Object();

5. Object *obj = new Object();
obj->intArray = new int[10];

6. Object **obj = new Object[n];
for (int i=0; i<n; i++) {
obj[i] = new Object();
obj[i]->intArray = new int[10];
}

In advance, thanks very much.

"Gus Gassmann" wrote in message
news:ba1d69e7-bd02-4c9a-9ab0-1f674fb3bc56@do4g2000vbb.googlegroups.com...
>
>I have been working with C++ for about four years now, mostly self-
>taught, and I get along reasonably well. However, last week I detected
>a memory leak in some code, and trying to locate it I realized that I
>do not know enough about destructors. I have created a number of use
>cases. How can I make sure in each case that the memory gets destroyed
>properly?

Of course this depens on the conext. But in general, use containers from the
standard library, or make your own containers to avoid the use of "naked"
new and delete.

>1. int *x[10];

std::vector<std::vector<int> > x;

>2. int *y = new int[10];

std::vector<int> y(10);


etc.

>3. Object *obj = new Object();
>
>4. Object **obj = new Object[n];
> for (int i=0; i<n; i++) obj[i] = new Object();
>
>5. Object *obj = new Object();
> obj->intArray = new int[10];
>
>6. Object **obj = new Object[n];
> for (int i=0; i<n; i++) {
> obj[i] = new Object();
> obj[i]->intArray = new int[10];
> }
>
>In advance, thanks very much.
>

Hi Gus

Gus Gassmann wrote:

> I have been working with C++ for about four years now, mostly self-
> taught, and I get along reasonably well. However, last week I detected
> a memory leak in some code, and trying to locate it I realized that I
> do not know enough about destructors. I have created a number of use
> cases. How can I make sure in each case that the memory gets destroyed
> properly?

Freds solution is the best because it is exception safe, but the
golden rule is:

when ever You use the keyword new it should be matched with the
keyword delete
and when You use new [] it should be matched with delete[]

1. int *x[10];
nothing.

2. int *y = new int[10];
delete[] y;

3. Object *obj = new Object();
delete obj;

etc.

But again Freds solution is better, because the vector takes care of the
deletion when the vector runs out of scoop, even if there is an exception.


Best regards
Asger-P

On Feb 7, 11:38*am, "Asger Joergensen" <J...@Asger-P.dk> wrote:
> Hi Gus
>
> Gus Gassmann wrote:
> > I have been working with C++ for about four years now, mostly self-
> > taught, and I get along reasonably well. However, last week I detected
> > a memory leak in some code, and trying to locate it I realized that I
> > do not know enough about destructors. I have created a number of use
> > cases. How can I make sure in each case that the memory gets destroyed
> > properly?
>
> Freds solution is the best because it is exception safe, but the
> golden rule is:
>
> when ever You use the keyword new it should be matched with the
> keyword delete
> and when You use new [] *it should be matched with delete[]
>
> 1. int *x[10];
> nothing.
>
> 2. int *y = new int[10];
> delete[] y;
>
> 3. Object *obj = new Object();
> delete obj;
>
> etc.
>
> But again Freds solution is better, because the vector takes care of the
> deletion when the vector runs out of scoop, even if there is an exception..

Some of the code is not written by me, so I am afraid Fred`s solution
does not work. Let me then summarize, just to be absolutely clear:

> 4. Object **obj = new Object[n];
> for (int i=0; i<n; i++) obj[i] = new Object();

for (int i=0; i<n; i++) delete obj[i];
delete[] obj;

> 5. Object *obj = new Object();
> obj->intArray = new int[10];

delete[] obj->intArray;
delete obj;

> 6. Object **obj = new Object[n];
> for (int i=0; i<n; i++) {
> obj[i] = new Object();
> obj[i]->intArray = new int[10];
> }

for (int i=0; i<n; i++) {
delete[] obj[i]->intArray;
delete obj[i];
}
delete[] obj;

Will that do it? Thanks!

Hi Gus

Gus Gassmann wrote:

> Some of the code is not written by me, so I am afraid Fred`s solution
> does not work. Let me then summarize, just to be absolutely clear:
....
>
> Will that do it?

It looks fine to me, but allocating and deleting memory inside an object
from the outside is really bad practice, You should at least ad a

void setIntSize( int size );
{
delete[] intArray;
intArray = new int[size]
}

to the object and then clean up in the destructor of the Object:

~Object()
{
delete[] intArray;
}

Do remember to zerro the intArray in the constructor.
Object(): intArray(0){}

Best regards
Asger-P

On 2/7/2012 12:34 PM, Asger Joergensen wrote:
> Hi Gus
>
> Gus Gassmann wrote:
>
>> Some of the code is not written by me, so I am afraid Fred`s solution
>> does not work. Let me then summarize, just to be absolutely clear:
> ...
>>
>> Will that do it?
>
> It looks fine to me, but allocating and deleting memory inside an object
> from the outside is really bad practice, You should at least ad a
>
> void setIntSize( int size );
> {
> delete[] intArray;
> intArray = new int[size]
> }
>
> to the object and then clean up in the destructor of the Object:
>
> ~Object()
> {
> delete[] intArray;
> }
>
> Do remember to zerro the intArray in the constructor.
> Object(): intArray(0){}

To the OP: Why not use std::vector? Or did you already answer that?

V
--
I do not respond to top-posted replies, please don't ask

On Feb 7, 6:53*am, Gus Gassmann <horand.gassm...@googlemail.com>
wrote:
> I have been working with C++ for about four years now, mostly self-
> taught, and I get along reasonably well. However, last week I detected
> a memory leak in some code, and trying to locate it I realized that I
> do not know enough about destructors. I have created a number of use
> cases. How can I make sure in each case that the memory gets destroyed
> properly?
>
> 1. int *x[10];
>
> 2. int *y = new int[10];
>
> 3. Object *obj = new Object();
>
> 4. Object **obj = new Object[n];
> * * for (int i=0; i<n; i++) obj[i] = new Object();
>
> 5. Object *obj = new Object();
> * * obj->intArray = new int[10];
>
> 6. Object **obj = new Object[n];
> * * for (int i=0; i<n; i++) {
> * * * * obj[i] = new Object();
> * * * * obj[i]->intArray = new int[10];
> * * }
>
> In advance, thanks very much.

Each call to new must be matched by exactly one call to delete, on the
exact same type of pointer, or a pointer to base class with a virtual
destructor. And the same for new[] and delete[] - each call to new[]
must be matched by exactly one call to delete[].

"Smart pointers" such as std::auto_ptr and std::unique_ptr, using
std::vector in place of new[] and delete[], using RAII in general,
etc., can help you ensure that you're not missing the matching calls
to delete and delete[].

Gus Gassmann <horand.gassmann@googlemail.com> wrote:
> How can I make sure in each case that the memory gets destroyed
> properly?

Since C++ does not have garbage collection of dynamically allocated
memory, the approach at making safe code is different from many other
languages.

As a general rule of thumb, if your code has 'new's and 'delete's
interprersed with other code, it's a big warning flag that you should
re-design your code. (Having 'new's and 'delete's in code, rather than
being encapsulated inside classes, usually means that your code is
error-prone and not exception-safe. It usually also means that it's more
complicated than it has to be.)

It's better if all dynamically allocated memory (and in fact, all
dynamically allocated resources in general) are managed by a class that
takes advantage of RAII. (There may be certain situations which are
exceptions to this, but they are rare.)

As suggested in the rest of the thread, the standard library already
offers you many classes that take care of managing memory for you, so
that you don't have to write a single 'new' or 'delete'. These are very
useful, and in fact I'd say at least 95% of dynamic memory management
in my own code is done using them.

But of course you shouldn't just blindly use them without understanding
how they work and what are their limitations, drawbacks and overhead.
(For example, using std::vector as a class member when a static array
would do makes your class significantly less efficient.)

While you can use the standard library containers and smart pointers
(especially since C++11) for something like at least 95% of the code, they
are not a panacea for all possible situations. There may be situations
where you just *have* to handle dynamically allocated memory yourself
(eg. if you need a data container type that the standard library does
not offer, or if you want to implement special management such as
copy-on-write or reference counting).

In these cases you should definitely take note on how the standard
library containers have been made (at least in terms of their constructors,
assignment and destructor). Learn the proper way of designing such a class
(google for "rule of three C++" for an idea).

It slows down as the list in other high level languages in basic operations.

The vector part in C++ offers auto-expansion and the boundary checking mechanism
absent from an array. For a list or a vector in C++ of tens of thousands of objects in non-heavy computing tasks it can save troubles.

But for digital images and videos of tens of millions of pixels to be processed
the C++ vector adds overheads, too.

On 02/ 8/12 08:39 PM, 88888 Dihedral wrote:
> It slows down as the list in other high level languages in basic operations.
>
> The vector part in C++ offers auto-expansion and the boundary checking mechanism
> absent from an array. For a list or a vector in C++ of tens of thousands of objects in non-heavy computing tasks it can save troubles.
>
> But for digital images and videos of tens of millions of pixels to be processed
> the C++ vector adds overheads, too.

No more than naked dynamic arrays.

--
Ian Collins

Ian Collins <ian-news@hotmail.com> writes:
> On 02/ 8/12 08:39 PM, 88888 Dihedral wrote:
>> But for digital images and videos of tens of millions of pixels to be
>> processed the C++ vector adds overheads, too.
>
> No more than naked dynamic arrays.

Well, possibly a little bit more (default initialisation, storing the
length, and possibly some address calculation if you want to address it
as if it were multidimensional, in zigzag fashion, say. Nothing you
couldn't fix with a templated custom class, though.)

And if those are the speed determining step of your algorithm, you're
doing it wrong.

On Feb 8, 6:51*pm, Gareth Owen <gwo...@gmail.com> wrote:
> Ian Collins <ian-n...@hotmail.com> writes:
> > On 02/ 8/12 08:39 PM, 88888 Dihedral wrote:
> >> But for digital images and videos of tens of millions of pixels to be
> >> processed the C++ vector adds overheads, too.
>
> > No more than naked dynamic arrays.
>
> Well, possibly a little bit more (default initialisation, storing the
> length, and possibly some address calculation if you want to address it
> as if it were multidimensional, in zigzag fashion, say. Nothing you
> couldn't fix with a templated custom class, though.)
>
> And if those are the speed determining step of your algorithm, you're
> doing it wrong.

Default initialization was a major overhead in some of my image
processing
code. I replaced std::vector with a home-grown dynamic_array<T> vector
like container that doesn't do default initialization. Problem solved.
It would be nice if it could be done with a custom allocator but you
can't.

Am 08.02.2012 07:38, schrieb Juha Nieminen:

> But of course you shouldn't just blindly use them without understanding
> how they work and what are their limitations, drawbacks and overhead.
> (For example, using std::vector as a class member when a static array
> would do makes your class significantly less efficient.)

This is just what happened today at work. We have a loop that iterates
over a buffer of 16 bit audio samples and uses a 64k look-up table for
each sample. The table was a class member vector<int16_t> with an
indexing function:

inline int16_t& lut(int16_t idx) { return table[idx]; }

That function was directly defined in the class definition, was marked
inline, and yet the compiler generated a call to it in the loop body.
When we marked that function with #pragma always_inline etc. the
compiler generated a warning that the function exceeded the speed/size
ratio, but that it would inline it anyway. When we looked at the
disassembly, there was still a call in the loop body, probably to the
operator[] of the vector (it was a long mangled name...).

Only when we changed the table to

int16_t table[65536];

the loop body became well optimized - even with the lut() function used.
And there was no need anymore to force it inline.

The code runs on a Blackfin DSP, and the compiler is Analog Devices'
blackfin compiler.

I was surprised because I thought the compiler should be able to inline
all the one-liners involved, both the lut() function and the vector's
indexing operator and properly optimize the loop. I was wrong...

No kidding about the "significantly less efficient" part.


Peter

Am 09.02.2012 00:30, schrieb Peter Remmers:

> inline int16_t& lut(int16_t idx) { return table[idx]; }

I wrote this from memory. Certainly, idx was not a signed type. I think
the samples are of some special fract16 type or something. Anyway, that
doesn't matter, as it's beside the point I wanted to make.


Peter

On Feb 8, 3:30*pm, Peter Remmers
<p.remm...@expires-2011-04-30.arcornews.de> wrote:
> Am 08.02.2012 07:38, schrieb Juha Nieminen:
>
> > * But of course you shouldn't just blindly use them without understanding
> > how they work and what are their limitations, drawbacks and overhead.
> > (For example, using std::vector as a class member when a static array
> > would do makes your class significantly less efficient.)
>
> This is just what happened today at work. We have a loop that iterates
> over a buffer of 16 bit audio samples and uses a 64k look-up table for
> each sample. The table was a class member vector<int16_t> with an
> indexing function:
>
> inline int16_t& lut(int16_t idx) { return table[idx]; }
>
> That function was directly defined in the class definition, was marked
> inline, and yet the compiler generated a call to it in the loop body.
> When we marked that function with #pragma always_inline etc. the
> compiler generated a warning that the function exceeded the speed/size
> ratio, but that it would inline it anyway. When we looked at the
> disassembly, there was still a call in the loop body, probably to the
> operator[] of the vector (it was a long mangled name...).
>
> Only when we changed the table to
>
> int16_t table[65536];
>
> the loop body became well optimized - even with the lut() function used.
> And there was no need anymore to force it inline.
>
> The code runs on a Blackfin DSP, and the compiler is Analog Devices'
> blackfin compiler.
>
> I was surprised because I thought the compiler should be able to inline
> all the one-liners involved, both the lut() function and the vector's
> indexing operator and properly optimize the loop. I was wrong...
>
> No kidding about the "significantly less efficient" part.

Well, all of the usual claims of good C++ efficiency with good C++
style assume a good compiler. Apparently this one needs some tuning...