will this cause memory leak?

[Yi]

There is originally a class called "message". For some reason, I need
to add "set<A> _update_list;" as a new
private data member of "message" ("A" can be IPv4 or IPv6, which are
defined elsewhere). In message.hh, I defined the following three
methods:

public:
void add_update(const A& this_ip) {
_update_list.insert(this_ip);
}

const set<A>& update_list() const {
return _update_list;
}

void copy_update_list(const set<A>& list) const {
_update_list = list;
}

private:
mutable set<A> _update_list;

Instead of changing the constructor, I added a statement explicitly
copying _update_list to a new object (using copy_update_list() )
everywhere a new message object is created by copying an existing
object.

add_update() is used to add IP addresses to the _update_list.

My questions is, if the message class has an empty destructor, would
the modification I did to the message class cause memory leak? (Would
_update_list be freed automatically?)

Thanks.

 

[Victor Bazarov]

There is originally a class called "message". For some reason, I need
to add "set<A> _update_list;" as a new
private data member of "message" ("A" can be IPv4 or IPv6, which are
defined elsewhere). In message.hh, I defined the following three
methods:

public:
void add_update(const A& this_ip) {
_update_list.insert(this_ip);
}

const set<A>& update_list() const {
return _update_list;
}

void copy_update_list(const set<A>& list) const {
_update_list = list;
}

private:
mutable set<A> _update_list;

Instead of changing the constructor, I added a statement explicitly
copying _update_list to a new object (using copy_update_list() )
everywhere a new message object is created by copying an existing
object.

add_update() is used to add IP addresses to the _update_list.

My questions is, if the message class has an empty destructor, would
the modification I did to the message class cause memory leak? (Would
_update_list be freed automatically?)

Yes, it will.

V

 

[Yi]

Then what's the right way to handle this?
Thanks.

 

[Andre Kostur]

There is originally a class called "message". For some reason, I need
to add "set<A> _update_list;" as a new
private data member of "message" ("A" can be IPv4 or IPv6, which are
defined elsewhere). In message.hh, I defined the following three
methods:

public:
void add_update(const A& this_ip) {
_update_list.insert(this_ip);
}

const set<A>& update_list() const {
return _update_list;
}

void copy_update_list(const set<A>& list) const {
_update_list = list;
}

private:
mutable set<A> _update_list;

Instead of changing the constructor, I added a statement explicitly
copying _update_list to a new object (using copy_update_list() )
everywhere a new message object is created by copying an existing
object.

Assuming you already have a constructor (and I presume you're talking about
a copy constructor), it's a better habit to use the initialization list in
the constructor rather than causing your member variable to be default
initialized followed by an assignment.

add_update() is used to add IP addresses to the _update_list.

My questions is, if the message class has an empty destructor, would
the modification I did to the message class cause memory leak? (Would
_update_list be freed automatically?)

Yes. When an object goes out of scope, it's destructor (in your case,
~message()) is invoked. Once it has completed, all of the destructors of
member variables will be invoked (in the reverse order of when their
constructors were invoked). In your case, set<A> will be destructed. As
part of that, each A in the set will be destructed. So assuming that A
behaves properly, everything will clean up nicely. The usual reminder here
is that the destructor for a pointer is a no-op. So if you really had set
<A*> as the type, each of the A* will be destructed (the pointer itself,
and _not_ what it's pointing to!).

 

[jichao06]

Then what's the right way to handle this?
Thanks.

What do you want to handle? By your code, there is a local copy of
_update_list for each message object. It is natual and safe to let the
destructor to desruct it.

JC

 

[Zeppe]

Then what's the right way to handle this?
Thanks.

The Victor reply was related to "will it be freed automatically"

Don't worry, your code will work properly. There is just one thing that
I can't figure out: you put 'mutable' in your variable, that is usually
evil. But I thought you did it because you needed to have the method
copy_update_list const, for some reason (I wonder why). Anyway, for
example, the method add_update is not const, so, I'm asking, do you
really need the set to be mutable. Remove it if it is not necessary.

Regards,

Zeppe

 

[=?ISO-8859-1?Q?Erik_Wikstr=F6m?=]

There is originally a class called "message". For some reason, I need
to add "set<A> _update_list;" as a new
private data member of "message" ("A" can be IPv4 or IPv6, which are
defined elsewhere). In message.hh, I defined the following three
methods:

public:
void add_update(const A& this_ip) {
_update_list.insert(this_ip);
}

const set<A>& update_list() const {
return _update_list;
}

void copy_update_list(const set<A>& list) const {
_update_list = list;
}

A small nitpick: names starting with an underscore are sometimes
reserved for the implementation (not the one you have above though) so
in general it's a good idea to avoid using names that begins with an
underscore.