vector and struct deallocation

Discussion in 'C++' started by Cristiano, Sep 1, 2012.

  1. Cristiano

    Cristiano Guest

    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
     
    Cristiano, Sep 1, 2012
    #1
    1. Advertising

  2. Cristiano

    Luca Risolia Guest

    On 01/09/2012 17:30, Paavo Helde wrote:

    > 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.
     
    Luca Risolia, Sep 1, 2012
    #2
    1. Advertising

  3. On 01.09.12 15.55, Cristiano wrote:
    > 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
     
    Marcel Müller, Sep 1, 2012
    #3
  4. Cristiano

    Cristiano Guest

    On 01/09/2012 17:30, Paavo Helde wrote:
    > 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
     
    Cristiano, Sep 1, 2012
    #4
  5. On 9/1/2012 1:39 PM, Cristiano wrote:
    > On 01/09/2012 17:30, Paavo Helde wrote:
    >> 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.


    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*.
    --
    I do not respond to top-posted replies, please don't ask
     
    Victor Bazarov, Sep 1, 2012
    #5
  6. Cristiano

    Luca Risolia Guest

    On 01/09/2012 19:39, Cristiano wrote:
    > On 01/09/2012 17:30, Paavo Helde wrote:
    >> 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?


    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.
     
    Luca Risolia, Sep 1, 2012
    #6
  7. Paavo Helde <> wrote:
    > Cristiano <> wrote in news:k1t44f$l7t$1@dont-
    >> 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]?
     
    Juha Nieminen, Sep 2, 2012
    #7
  8. Cristiano

    Cristiano Guest

    On 02/09/2012 10:18, Paavo Helde wrote:
    > Juha Nieminen <> wrote in news:k1uq68$s4d$1
    > @speranza.aioe.org:
    >
    >> Paavo Helde <> wrote:
    >>> Cristiano <> wrote in news:k1t44f$l7t$1@dont-
    >>>> 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, Sep 2, 2012
    #8
  9. Cristiano

    Cristiano Guest

    On 01/09/2012 20:25, Luca Risolia wrote:
    > 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, Sep 2, 2012
    #9
  10. Cristiano

    Cristiano Guest

    On 01/09/2012 20:19, Victor Bazarov wrote:
    > 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
     
    Cristiano, Sep 2, 2012
    #10
  11. Paavo Helde <> wrote:
    > 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.)
     
    Juha Nieminen, Sep 2, 2012
    #11
  12. On Sep 2, 10:43 am, Cristiano <> wrote:
    > On 02/09/2012 10:18, Paavo Helde wrote:
    >
    >
    >
    >
    >
    >
    >
    >
    >
    > > Juha Nieminen <> wrote in news:k1uq68$s4d$1
    > > @speranza.aioe.org:

    >
    > >> Paavo Helde <> wrote:
    > >>> Cristiano <> wrote innews:k1t44f$l7t$1@dont-
    > >>>> 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[]
     
    Nick Keighley, Sep 2, 2012
    #12
  13. Cristiano

    Luca Risolia Guest

    On 02/09/2012 11:50, Cristiano wrote:
    > On 01/09/2012 20:25, Luca Risolia wrote:
    >> Assuming that std::vector is the right container for your needs,

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

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


    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... :)]


    :)
     
    Luca Risolia, Sep 2, 2012
    #13
  14. Cristiano

    Cristiano Guest

    On 02/09/2012 13:57, Luca Risolia wrote:
    > On 02/09/2012 11:50, Cristiano wrote:
    >> On 01/09/2012 20:25, Luca Risolia wrote:
    >>> Assuming that std::vector is the right container for your needs,

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


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

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

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

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


    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, Sep 2, 2012
    #14
  15. Cristiano

    Cristiano Guest

    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
     
    Cristiano, Sep 2, 2012
    #15
  16. Cristiano

    Rui Maciel Guest

    Rui Maciel, Sep 2, 2012
    #16
  17. On 9/2/2012 12:42 PM, Cristiano wrote:
    > On 02/09/2012 13:57, Luca Risolia wrote:
    >> On 02/09/2012 11:50, Cristiano wrote:
    >>> On 01/09/2012 20:25, Luca Risolia wrote:
    >>>> Assuming that std::vector is the right container for your needs,
    >>>
    >>> 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/

    >
    > I think that std:vector should be fine for me.
    >
    >>>> 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.

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


    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
    --
    I do not respond to top-posted replies, please don't ask
     
    Victor Bazarov, Sep 2, 2012
    #17
  18. On 9/2/12 2:46 PM, Rui Maciel wrote:
    > Juha Nieminen wrote:
    >
    >> 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).
     
    Richard Damon, Sep 3, 2012
    #18
  19. Cristiano

    ptyxs Guest

    Le 02/09/2012 18:52, Cristiano a écrit :
    > 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

    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 ??
     
    ptyxs, Sep 3, 2012
    #19
  20. Cristiano

    Guest

    On Sunday, September 2, 2012 6:52:41 PM UTC+2, Cristiano wrote:
    > 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.
     
    , Sep 3, 2012
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Andy Chang

    Vector pointer and deallocation

    Andy Chang, Jan 4, 2005, in forum: C++
    Replies:
    7
    Views:
    8,206
    Ron Natalie
    Jan 5, 2005
  2. Chris Fogelklou
    Replies:
    36
    Views:
    1,391
    Chris Fogelklou
    Apr 20, 2004
  3. Nicolas Matringe

    std.textio, readline and memory deallocation

    Nicolas Matringe, Sep 1, 2006, in forum: VHDL
    Replies:
    9
    Views:
    2,105
    Paul Uiterlinden
    Sep 4, 2006
  4. Replies:
    8
    Views:
    1,930
    Csaba
    Feb 18, 2006
  5. madhu
    Replies:
    6
    Views:
    641
    madhu
    Nov 13, 2006
Loading...

Share This Page