Question about destructors

[Gus Gassmann]

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.

[Fred Zwarts (KVI)]

"Gus Gassmann" wrote in message
news:ba1d69e7-bd02-4c9a...@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.

[Asger Joergensen]

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

[Gus Gassmann]

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!

[Asger Joergensen]

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

[Victor Bazarov]

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

[Joshua Maurice]

On Feb 7, 6:53 am, Gus Gassmann <horand.gassm...@googlemail.com>
wrote:

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[].

[Juha Nieminen]

Gus Gassmann <horand....@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).

[88888 Dihedral]

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.

[Ian Collins]

No more than naked dynamic arrays.

--
Ian Collins

[Gareth Owen]

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.

[Gert-Jan de Vos]

On Feb 8, 6:51 pm, Gareth Owen <gwo...@gmail.com> wrote:

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.

[Peter Remmers]

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

[Peter Remmers]

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

[Joshua Maurice]

On Feb 8, 3:30 pm, Peter Remmers

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

[SG]

On Feb 7, 6:13 pm, 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:
>
> > 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?

Not really. This is pretty fragile C++ code and doomed to break
eventually. For example, it's not exception-safe. Try to exploit the
strengths of C++. One of the biggest corner stones of modern C++
design is letting objects "own"/take care of other resources. C++
allows you to delegate the responsibility of cleaning up things so you
don't have to do it yourself.

Fragile:
int* p = new int[10];
...
delete[] p;

Good:
vector<int> v (10);
...

To put it differently: If you want memory to be released if a pointer
goes out of scope, the raw pointer type would be a misuse because it
doesn't have the semantics you actually want.

Instead of new[] and delete[], prefer std::vector. Instead of "naked"
new, prefer the use of std::unique_ptr (or std::shared_ptr) so that
every heap-allocated object is "owned" by at least one other object.
This makes "delete" in user code rather obsolete. And even "new"
should be avoided in preference of std::make_shared and
std::make_unique (make_unique is not yet standard but you can write it
yourself

template<class T, class...Args>
std::unique_ptr<T> make_unique(Args&&...args)
{
return {new T(std::forward<Args>(args)...)};
}

). In case you are worried about unnecessary copying with respect to
vectors and functions returning function-local vectors by value,
you'll be pleased to know that C++2011 will make sure this is about as
efficient as returning a raw pointer.

Cheers!
SG

[88888 Dihedral]

在 2012年2月10日星期五UTC+8下午8时35分19秒，SG写道：

In C++ objects in classes with several vectors or other containers
are allowed. Even copying an object is not so trivial.

[Jorgen Grahn]

On Thu, 2012-02-09, Joshua Maurice wrote:
> On Feb 8, 3:30 pm, Peter Remmers
> <p.remm...@expires-2011-04-30.arcornews.de> wrote:

...

>> 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]; }

...

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

...
>> 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...

Or perhaps some debug version of std::vector was used.

/Jorgen

--
// Jorgen Grahn <grahn@ Oo o. . .
\X/ snipabacken.se> O o .