vector and struct deallocation

[Cristiano]

I use these two structures:

struct ROUTE {
char *name;
char *Pid;
char *Nid;
};

struct WPT {
char ID[6];
ROUTE *route;
};

and

std::vector <WPT *> wpt;

I do:
WPT *wp= new WPT; wpt.push_back(wp);

ROUTE *rou= new ROUTE; wp->route= rou;
rou->name= new char[strlen(RouteName) + 1];
strcpy(rou->name, RouteName);

and the same for Pid and Nid:

rou->Pid= new char[strlen(Pid+ 1]; strcpy(rou->Pid, Pid);
rou->Nid= new char[strlen(Nid+ 1]; strcpy(rou->Nid, Nid);

When I no longer need that stuff, I do:

for(int i= 0; i < wpt.size(); i++) {
if(wpt->route) {
delete[] wpt->route->name;
delete[] wpt->route->Pid;
delete[] wpt->route->Nid;
delete wpt->route;
}
delete wpt;
}

The debugger (I use Visual Studio 2008 EE) tell me that there is a
memory leak.

Is the deallocation procedure correct?

Thanks
Cristiano

 

[Luca Risolia]

std::vector<WPT> wpt;

...or a vector of smart pointers (std::shared_ptr's, for example) to WPT
if you really need a vector of pointers.

See, the question of proper deallocation does not even arise here.

...the same consideration applies to smart pointers.

 

[Marcel Müller]

struct ROUTE {
char *name;
char *Pid;
char *Nid;
};

Don't use char* in C++ code unless you like application crashes and
memory leaks. Use std::string for strings only. (const char* for string
constants is fine.)

std::vector <WPT *> wpt;

I do:
WPT *wp= new WPT; wpt.push_back(wp);

Don't use raw pointers with new for the same reason.
Use smart pointers instead. boost::shared_ptr will most likely fit your
needs.


Marcel

 

[Cristiano]

Why? What's wrong with:

struct Route {
std::string name, pid, nid;
};

struct WPT {
std::string id;
Route route;
};

std::vector<WPT> wpt;

See, the question of proper deallocation does not even arise here.

I've never used std:string.
I implemented your code and it works, but considering that there are
around 30,000 to 50,000 WPT and that the "real" structures are:

struct ROUTE {
short type;
std::string name;
std::string Pid; short Pmea;
std::string Nid; short Nmea;
};

struct WPT {
std::string ID;
char type;
double lat, lon;
short var;
std::vector<ROUTE> route;
};

there could be any speed penalty or memory problems with those structures?

Thank you
Cristiano

 

[Victor Bazarov]

I've never used std:string.

There is always the first time for everything.

I implemented your code and it works, but considering that there are
around 30,000 to 50,000 WPT and that the "real" structures are:

struct ROUTE {
short type;
std::string name;
std::string Pid; short Pmea;
std::string Nid; short Nmea;
};

struct WPT {
std::string ID;
char type;
double lat, lon;
short var;
std::vector<ROUTE> route;
};

there could be any speed penalty or memory problems with those structures?

Before any yellow-bellied newbie starts to argue for the 'yes' answer to
that, let me quote the old wisdom: "first make it work, THEN make it
fast". When you write code, there will always be some kind of
performance "penalty". Code takes some time to run. Now, library code
is developed by people who are much better programmers than you and I.
Using their code means you can program more efficiently, get your code
to work sooner, make it more extensible and reusable. And it can be a
tiny bit slower or have a slightly larger footprint than, say, the same
functionality written in assembly.

It's called "trade-off". If you keep your code simple and easy to
understand and maintain, you will be able to speed it up at some point,
hopefully. But if you write spaghetti that you yourself barely can make
out, or get all the bugs out of, what speed improvement can be worth all
the trouble of keeping that code functional? Or do you practice
"write-only code"? "Fire and forget"?

You could compare the code that uses Standard library strings and
containers to your manual naked pointer allocation/deallocation, but you
have to get *your* code working first. Once you get there (and at this
point I am not really sure you have it all together), you will probably
see that the "performance penalty" is either negligible, or you're not
measuring it right. Is it necessary to go into trouble of "reinventing
the wheel" and writing your own allocations for what seems to be a bunch
of simple strings and an array of them (vector)?

Get yourself a good book on Standard Library. Nicolai Josuttis just
released the second edition of his great tutorial. Once you get it,
*study*. The biggest problem novices have is *believing that they know
enough*. Sorry, my friends, you *don't*. Yet. And it can only be
corrected by a *systematic learning*. So, tap into the good sources,
and feed your brain, it will appreciate it. Good luck!

V

P.S. Doubt is a good thing. If you doubt the performance, test it,
don't assume, and don't trust anybody's saying that "it's as fast" or
"it carries a penalty". People will tell you that because they want you
to respect them, or trust them, or for some other selfish reasons. The
only sensible thing to do in that situation, however, is to *measure*
the performance and compare it to the *requirements* that you establish
*ahead of time*. So, make sure you have the standard to which you want
to compare, and make sure that what you measure actually *matters*.

 

[Luca Risolia]

I've never used std:string.
I implemented your code and it works, but considering that there are
around 30,000 to 50,000 WPT and that the "real" structures are:

struct ROUTE {
short type;
std::string name;
std::string Pid; short Pmea;
std::string Nid; short Nmea;
};

struct WPT {
std::string ID;
char type;
double lat, lon;
short var;
std::vector<ROUTE> route;
};

there could be any speed penalty or memory problems with those structures?

Assuming that std::vector is the right container for your needs, to
avoid unnecessary memory reallocations, don't forget to consider
std::vector::reserve before starting to push your elements into the
vector, since you already know its maximum size.
Also, don't forget to use std::move if you can move your objects (if
lvalues) into the vector. Due to the high number of elements you plan to
store, the above two tips should make some difference.

 

[Juha Nieminen]

struct WPT {
char ID[6];
ROUTE *route;

struct WPT {
std::string id;
Route route;

If the 'id' member can have at most 6 characters, and especially if this
is checked when it's assigned to, why on earth would you want to use
std::string instead of char[6]?

 

[Cristiano]

struct WPT {
char ID[6];
ROUTE *route;

struct WPT {
std::string id;
Route route;

If the 'id' member can have at most 6 characters, and especially if this
is checked when it's assigned to, why on earth would you want to use
std::string instead of char[6]?

Why not? std::string is easier to work with than a raw array. For example
as it has operator==, I can easily test if(wpt.id=="123456") instead of
using memcmp() or strcmp() or whatever is more appropriate. Also the
assumptions like "'id' member can have at most 6 characters" are often
prone to failing in some more or less distant future.

In my case there could be 1 to 5 null terminated chars.

I don't know how the std::string is stored in that struct, but I suppose
that the memory waste is much bigger than char ID[6]; is that right?
I use an entire class just to do: id.assign("1234") and id=="xxxx".
The most important thing in my code is the speed.

Cristiano

 

[Cristiano]

Assuming that std::vector is the right container for your needs,

What could I use?

to avoid unnecessary memory reallocations, don't forget to consider
std::vector::reserve before starting to push your elements into the
vector, since you already know its maximum size.

No, I don't know.

Also, don't forget to use std::move if you can move your objects (if
lvalues) into the vector. Due to the high number of elements you plan to
store, the above two tips should make some difference.

May be I'll need to sort the vector, but I don't know at this moment.

[Certo che usare il mio "inglese" per ragionare con un italiano, mi fa
strano... ]

Cristiano

 

[Cristiano]

Before any yellow-bellied newbie starts to argue for the 'yes' answer to
that, let me quote the old wisdom: "first make it work, THEN make it
fast". When you write code, there will always be some kind of
performance "penalty". Code takes some time to run. Now, library code
is developed by people who are much better programmers than you and I.
Using their code means you can program more efficiently, get your code
to work sooner, make it more extensible and reusable. And it can be a
tiny bit slower or have a slightly larger footprint than, say, the same
functionality written in assembly.

I agree, good philosophy!

Get yourself a good book on Standard Library.

I have two old books, but what I really do since 1982 is to write C code
and some years later I added few C++ "extensions".
Some years ago I "added" the std:vector, now std:string; not bad!

Cristiano

 

[Juha Nieminen]

Why not? std::string is easier to work with than a raw array. For example
as it has operator==, I can easily test if(wpt.id=="123456") instead of
using memcmp() or strcmp() or whatever is more appropriate. Also the
assumptions like "'id' member can have at most 6 characters" are often
prone to failing in some more or less distant future.

Unless the particular std::string implementation happens to use short
string optimization (which really isn't a given), it will be horribly
inefficient and wastes memory. Even if it *does* implement short string
optimization, it will still be a lot larger than 6 bytes. std::string
doesn't protect you from out-of-bounds accesses either (except in the
debug mode of some compilers).

If you really need an operator== for it, either write it as a member of
that struct or use std::array instead.

There's an advantage to std::string over a static array only if the size
of the string could grow. (Even then you should be aware of the possible
efficiency problems.)

 

[Nick Keighley]

@speranza.aioe.org:

struct WPT {
     char ID[6];
     ROUTE *route;
struct WPT {
        std::string id;
        Route route;
If the 'id' member can have at most 6 characters, and especially if this
is checked when it's assigned to, why on earth would you want to use
std::string instead of char[6]?

Why not? std::string is easier to work with than a raw array. For example
as it has operator==, I can easily test if(wpt.id=="123456") instead of
using memcmp() or strcmp() or whatever is more appropriate. Also the
assumptions like "'id' member can have at most 6 characters" are often
prone to failing in some more or less distant future.

In my case there could be 1 to 5 null terminated chars.

I don't know how the std::string is stored in that struct, but I suppose
that the memory waste is much bigger than char ID[6]; is that right?
I use an entire class just to do: id.assign("1234") and id=="xxxx".
The most important thing in my code is the speed.

but how fast is fast enough? Does it load this stuff from a file? How
big is the file likely to be (K, M, G?)? Is this a "hard" real time
system (something *must* happen within a specified time or the
aeroplane crashes)- most system aren't. In most applications you'll
have a hard time telling the difference between std::string and char[]

 

[Luca Risolia]

What could I use?

A way to choose the ideal container is to list all the possible
containers and compare the costs associated with the most frequent
operations you plan to perform on the container itself.
A comparison chart is here: https://www.cplusplus.com/reference/stl/

No, I don't know.

You said the number of WPT's may vary from "30000 to 50000" ..

May be I'll need to sort the vector, but I don't know at this moment.

If you don't know now, then maybe sorting is not a priority. In my
experience "small" vector's are usually still ideal even if you plan to
sort them often, also because they can be easily cached, but it's hard
to say when you end up having thousands of those WPT's.

[Certo che usare il mio "inglese" per ragionare con un italiano, mi fa
strano... ]

 

[Cristiano]

A way to choose the ideal container is to list all the possible
containers and compare the costs associated with the most frequent
operations you plan to perform on the container itself.
A comparison chart is here: https://www.cplusplus.com/reference/stl/

I think that std:vector should be fine for me.

You said the number of WPT's may vary from "30000 to 50000" ..

I meant that usually I'll load about 40000 WPT's, but I can load only
9724 WPTs or even 68351 or 52342; I don't know in advance.

If you don't know now, then maybe sorting is not a priority. In my
experience "small" vector's are usually still ideal even if you plan to
sort them often, also because they can be easily cached, but it's hard
to say when you end up having thousands of those WPT's.

It is possible that I'll need to sort them (to speed up the search using
the binary search) only when the program starts.

Cristiano

 

[Cristiano]

Is the deallocation procedure correct?

I would like to thank all of you, but nobody answered my question.
Now I use a totally different approach, but it could be still
interesting to know the correct deallocation procedure.

Cristiano

 

[Rui Maciel]

If the 'id' member can have at most 6 characters, and especially if this
is checked when it's assigned to, why on earth would you want to use
std::string instead of char[6]?

http://www.parashift.com/c++-faq/use-string-not-char-ptr.html


Rui Maciel

 

[Victor Bazarov]

I think that std:vector should be fine for me.


I meant that usually I'll load about 40000 WPT's, but I can load only
9724 WPTs or even 68351 or 52342; I don't know in advance.

Another container to consider: std::deque. It has the advantage of
being faster than vector on appends (both front and back). Once you
filled it up, and you know how many objects you have, you can repackaged
your 'deque' into a 'vector' (for sorting, etc.)

[..]

V

 

[Richard Damon]

If the 'id' member can have at most 6 characters, and especially if this
is checked when it's assigned to, why on earth would you want to use
std::string instead of char[6]?

http://www.parashift.com/c++-faq/use-string-not-char-ptr.html


Rui Maciel

That shows why std:string is better than raw char* pointers, not better
than char[] arrays.

If, as the example seems to indicate, these labels really are strictly
length limited, short labels, that are mostly just assigned on creation,
or maybe changed later on via a UI, then I would think that a char[]
array to store them may well be best choice.

I would NOT put in a fixed number (like the 6 proposed), but use a
manifest constant of some form to declare the size, so that current code
can use that constant where it needs the size, and if in the future, it
is decided to change it, you don't need to search for all uses of it.

I would also likely hide the storage in a private variable and provide
member functions to do the operations you need on it, so that in the
future if you DO need to change the storage method, the changes are very
localized. (Basic rule, clients of a class want to know what the class
does, not how it does it). In general, most member variables of a class
tend to be private, or occasionally just protected, only very rarely
public (and then don't just arbitrarily add get/set routines for every
member variable).

 

[ptyxs]

Le 02/09/2012 18:52, Cristiano a écrit :

I would like to thank all of you, but nobody answered my question.
Now I use a totally different approach, but it could be still
interesting to know the correct deallocation procedure.

Cristiano

In one of the first messages of this thread, Paavo Helde gave an
alternative code in which all the allocation/deallocation job was done
tranparently by C++ tools. In what sense did that message NOT answer
your question ??

 

[goran.pusic]

On 01/09/2012 15:55, Cristiano wrote: > Is the deallocation procedure correct? I would like to thank all of you, but nobody answered my question. Now I use a totally different approach, but it could be still interesting to know the correct deallocation procedure. Cristiano

I wanted to write a long-winded explanation of what you likely needed to do, but gave up because I saw "use std::string" advice. That is likely all you need, really. About speed: just saying "it's the most important thing", is useless, because it actually shows you don't have any particular requirement.

Still, about the correct deallocation procedure... It all depends, really, and it depends on what you decide to do with your structures. If you put them into a vector, and you use heap storage ("new char[X]"), it is best, by a long far, to apply the rule of three (http://en.wikipedia.org/wiki/Rule_of_three_(C++_programming)). Study it, hard. To my mind, this isn't evena C++ rule, it's a C rule (with a twist that you need to "enforce", through code, existence and consistent use of a ctor, dtor and the assignment operator). Rule of three is a simple answer to a question of heap object ownership (question is: "Who "owns" any of heap objects in code, at ANY point inprogram lifetime", or, said differently, "--When-- do I delete any of objects I new-ed?")

Goran.