can I use stl vector iterator to delete a vector of pointers?

Discussion in 'C++' started by zl2k, Sep 2, 2010.

  1. zl2k

    zl2k Guest

    hi, there,

    Assume I have a vector of points vector<obj*> v and I need to free the
    memory.
    More over, some of the element in v are already deleted (null
    pointer).

    Is it ok if I do like this:

    for (vector<Obj*>::iterator it(v.begin()), ite(v.end()); it!= ite; +
    +it{
    if (*it != 0){
    delete *it;
    }
    }

    Thanks a lot.

    zl2k
     
    zl2k, Sep 2, 2010
    #1
    1. Advertisements

  2. zl2k

    Kai-Uwe Bux Guest

    a) Deleting the pointers will not invalidate the iterator. Thus, the idea in
    itself is sound.

    b) The check for 0 before the delete is unnecessary: deleting a 0 pointer is
    a null-op.

    c) You might want to do "*it = 0" after the delete: for one thing, you seem
    to be using the convention of signalling by 0 that memory has been
    deallocated (although that is a questionable practice); also it can be
    problematic to keep singular pointer values inside the vector: when the
    vector reallocates those values will be moved around and that causes
    undefined behavior.


    Best

    Kai-Uwe Bux
     
    Kai-Uwe Bux, Sep 2, 2010
    #2
    1. Advertisements

  3. Looks fine, but can be extremely simplified (apart that you mistyped
    something up there):

    for(int i = 0; i < v.size(); ++i) {
    delete v;
    }

    You can safely call delete on a null pointer.
     
    Francesco S. Carta, Sep 2, 2010
    #3
  4. Having invalid pointers into a vector of pointers and reallocating the
    vector really does call for UB? I'm not sure I'm catching the point
    (I've read that assertion before, but I still haven't wrapped my mind
    around this, I'd like to make it clear once for all).

    Besides, the advice of zeroing the pointer is good, but the best would
    be to simply drop the vector or to clear it out, since all the pointers
    it contains are dangling or null, by now.
     
    Francesco S. Carta, Sep 2, 2010
    #4
  5. zl2k

    Kai-Uwe Bux Guest

    It does in C++03. I am not sure about C++0x as that will have move
    semantics. As of now, reallocating a vector involves assignment or copy-
    construction, either of which necessitate an lvalue-rvalue conversion (maybe
    a move will do that too). Although the language in [4.1/1] leaves out
    singular values, it seems pretty clear that the intent is to make it UB to
    invoke an lvalue-rvalue conversion on an invalid pointer. Under the hoods,
    that happens when a vector of pointers reallocates and an invalid pointer is
    involved.

    There is some discussion / disagreement / open question as to whether
    _purely and absolutely formally_ even the mere existence of singular values
    in a pointer triggers undefined behavior. A common sense reaction is,
    however, to not bother unless actual operations on the vector are performed
    that may reasonably invoke UB. This suffices to stay clear of UB on all
    implementations known to man (or at least in this forum, nobody ever pointed
    out a less sane implementation).
    I felt the urge to point out possible pitfalls since we do not know what
    happens to the vector afterwards. I agree that the cleanest would be, to
    clear out the vector or to have it destroyed.


    Best

    Kai-Uwe Bux
     
    Kai-Uwe Bux, Sep 2, 2010
    #5
  6. I see, I've also read some threads I've found hanging here and there, it
    seems that this issue gets raised from time to time and the debates that
    it generates might not be worth the time spent on it - as the practical
    advantages of such a case being "well defined" would be around zero.

    Now I've understood the issue and I'm happy considering as "definitive
    UB" the fact of doing anything with a dangling pointer other than
    zeroing it or assigning a valid pointer to it, thank you for your
    explanation Kai-Uwe.
     
    Francesco S. Carta, Sep 2, 2010
    #6
  7. zl2k

    Daniel Pitts Guest

    Dereferencing an invalid pointer is not the only UB with regards to
    invalid pointers. Simply reading the value is UB, AIUI.

    The only defined operations on an invalid pointer: set it to null, or
    set it to a valid location.
     
    Daniel Pitts, Sep 2, 2010
    #7
  8. zl2k

    Kai-Uwe Bux Guest

    It does not. However, dereferencing an invalid pointer is not the only way
    of using an invalid pointer to yield UB. See [4.1/1].

    Consider:

    int* p = new int();
    delete p;
    int* q ( p );

    That is UB even though p is not dereferenced. Similarly, and for essentially
    the same reason, the following is UB:

    int* p;
    int* q ( p );
    Still UB. If you write a pointer to the union, even reading the value as an
    int is UB. If the pointer value is singular, copying is UB even if the
    value, interpreted as an int, would not be singular. The point is that the
    union at this point holds an object of type int* with a singular value. That
    triggers [4.1/1]. Anyhow, a conforming program can not see that the value as
    an int would be regular for reading that value is UB.


    Best

    Kai-Uwe Bux
     
    Kai-Uwe Bux, Sep 2, 2010
    #8
  9. zl2k

    Ian Collins Guest

    In addition to the other replies, sizeof(int) != sizeof(void*) on most
    64 bit architectures.
     
    Ian Collins, Sep 3, 2010
    #9
  10. You said : also it can be
    problematic to keep singular pointer values inside the vector

    How can this be solved?
    ankur
     
    ankur.chaplot, Sep 3, 2010
    #10
  11. zl2k

    Kai-Uwe Bux Guest

    wrote:

    [...]
    By not doing it :)

    Depending on the case, this ranges from easy to very hard. The easy case is
    when the vector contains pointers whose values occur nowhere else in the
    program (or the vector is the sole owner of all pointees of its elements).
    In this case, you have control over invalidating operations and you can
    remove singular values from the vector as (or before) they arise.

    Problematic is when pointer values inside the vector can be invalidated by
    operations elsewhere. This is a general problem with pointers. If you have
    two pointer object sharing the same value, calling delete on one of them
    also invalidates the value of the other. Even if you destroy or null the
    pointer which you delete, the other hangs around just waiting to cause UB.
    This is especially tricky as you have no way of telling by looking at a
    pointer whether its value is singular. So, the best thing is to take care of
    all "aliases" of a pointer at the time you want to delete one of them. How
    to handle this problem, however, depends on the application. The main lesson
    here is that the vector<T*> case is not really special.


    Best

    Kai-Uwe Bux
     
    Kai-Uwe Bux, Sep 3, 2010
    #11
  12. zl2k

    Goran Pusic Guest

    I know of no implementation where this won't work, but I think a
    better thing is:

    // Everyone and their mother already have
    // a variant of code below, right? RIGHT? ;-)
    template<typename ptr>
    void del_null(ptr& val)
    {
    ptr temp = val;
    val = 0;
    delete temp;
    }

    template<class container>
    void del_null_container_elements(container& c)
    {
    std::for_each(c.begin(), c.end(), &del_null<container::value_type>);
    }

    template<class container>
    void del_null_container_elements(container& c)
    {
    delete_container_elements(c);
    c.clear();
    }

    Goran.
     
    Goran Pusic, Sep 3, 2010
    #12
  13. It is not. As other said, it is UB.

    Better way would be to use something like SharedPtr instead of raw
    pointer. Then you can remove the element, and let the smart pointer do
    the deallocation.
     
    Vladimir Jovic, Sep 3, 2010
    #13
  14. What "it is not", and what "it is UB", regarding the text you quoted
    from me? What I suggested above is exactly about _avoiding_ UB.
    Smart pointers are a good suggestion.
     
    Francesco S. Carta, Sep 3, 2010
    #14
  15. [Snipped too much - restoring now]
    Advise to use raw pointers is bad. Specially to do the suggestion c, as
    that is UB (nicely explained by Kai-Uwe Bux). It would most likely work
    as expected, but using raw pointers complicates code.
     
    Vladimir Jovic, Sep 3, 2010
    #15
  16. Ah, I understand now. Yes, raw pointers are dangerous, but are sometimes
    a necessary evil, paraphrasing the C++ FAQ.

    I think I've found something interesting about all of this stuff, I'll
    post it as a separate topic because I think it needs the appropriate
    attention.
     
    Francesco S. Carta, Sep 3, 2010
    #16
  17. * Kai-Uwe Bux, on 02.09.2010 20:59:
    Here the term "singular value" is used to mean "invalid pointer", as it also is
    some places in the standard.

    However AFAICS it's not defined in the standard.

    Is it defined? And what the heck does it /mean/ (that leads to its use for
    invalid pointer or iterator values)?


    Cheers,

    - Alf
     
    Alf P. Steinbach /Usenet, Sep 3, 2010
    #17
  18. .... or destroy it, probably. :)
    (Otherwise, { int* p; } would invoke UB... or does it?)

    So, do we really need to set the pointers to null before destroying
    the vector?
     
    Seungbeom Kim, Sep 6, 2010
    #18
  19. No it doesn't, because there is no lvalue to rvalue conversion in your
    example.
    As it seems, formally, yes [*], we need to nullify them, because the
    actions performed by the vector during reallocation or during
    destruction might lead to that lvalue to rvalue conversion, thus calling
    UB into play.

    In practice there seems to be no danger on most architectures, but read
    all the other posts to get deeper insight, and read the "UB while
    dealing with invalid raw pointers, the std::uninitialized_fill case"
    thread too (Bo Persson's posts in particular).

    [*] In layman terms, the point is that a pointer pointing to
    appropriately allocated space (even if that space doesn't contain any
    valid object) is safe to be copied and compared (but not dereferenced),
    while a pointer pointing to {unallocated/random/deallocated} space is a
    timed bomb. But once more, see the other posts from more knowledgeable ones.
     
    Francesco S. Carta, Sep 6, 2010
    #19
  20. zl2k

    James Kanze Guest

    It doesn't. But it does require an lvalue to rvalue conversion
    of the right hand operand. And an lvalue to rvalue conversion
    of an invalid pointer is undefined behavior (and could trigger
    a program crash, at least on some processors I've used in the
    past).
    Formally: if the last assigned value was a pointer, and that
    pointer has subsequently been invalidated, it's undefined
    behavior; otherwise, it's OK. Practically, the constraints on
    unions were designed precisely to ensure that a compiler could
    always do assignment using bitwise copy, ignoring the type of
    the last assigned value, so it will always be OK.
     
    James Kanze, Sep 6, 2010
    #20
    1. Advertisements

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.