Dangerous iterator usage after erase()-method.

Bug #812018 reported by Reijo Tomperi on 2011-07-17
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Inkscape
Medium
Kris

Bug Description

With revision 10464 I found error (using iterator which was invalidated by erase()-method) from following files:

[../inkscape/inkscape/src/dom/stylesheets.h:128]: (error) Dangerous iterator usage after erase()-method.
[../inkscape/inkscape/src/dom/css.h:643]: (error) Dangerous iterator usage after erase()-method.
[../inkscape/inkscape/src/dom/events.h:519]: (error) Dangerous iterator usage after erase()-method.
[../inkscape/inkscape/src/dom/events.h:578]: (error) Dangerous iterator usage after erase()-method.

# Similar error, fixed in practice, but it would better to use return value of erase() instead so it would be fixed in theory also.
[../inkscape/inkscape/src/ui/tool/path-manipulator.cpp:1006]: (error) Dangerous iterator usage after erase()-method.

Fix it rather simple, modify loop and use the return value of erase(). Here is an example code to demonstrate the error and fix:

// Example of the error (should segfault if executed)
#include <vector>
int main()
{
    std::vector<int> items;
    items.push_back(1);
    items.push_back(2);
    items.push_back(3);
    std::vector<int>::iterator iter;
    for (iter=items.begin() ; iter!=items.end() ; iter++)
    {
        if (true)
        {
            items.erase(iter);
        }
    }
}

// Fixed version:
#include <vector>
int main()
{
    std::vector<int> items;
    items.push_back(1);
    items.push_back(2);
    items.push_back(3);
    std::vector<int>::iterator iter;
    for (iter=items.begin(); iter!= items.end();)
    {
        if (true)
        {
            iter = items.erase(iter);
        }
        else
        {
            ++iter;
        }
    }
}

There is no real life example for this bug as it was found when testing Cppcheck against Inkscape code, but code like this can easily cause crash so it should be fixed.

su_v (suv-lp) on 2011-07-18
tags: added: cppcheck
su_v (suv-lp) on 2011-07-19
Changed in inkscape:
assignee: nobody → Jon A. Cruz (jon-joncruz)
status: New → Confirmed
Kris (kris-degussem) on 2011-10-29
Changed in inkscape:
assignee: Jon A. Cruz (jon-joncruz) → Kris (kris-degussem)
importance: Undecided → Medium
milestone: none → 0.49
Kris (kris-degussem) on 2011-10-29
Changed in inkscape:
status: Confirmed → In Progress
Kris (kris-degussem) wrote :

Fix committed in revision 10706.
Thanks for the self explaining code example Reijo!

Changed in inkscape:
status: In Progress → Fix Committed
Bryce Harrington (bryce) on 2015-02-21
Changed in inkscape:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers