Re: problem with a c++ construct in calc, fdo#54498

Hi,

On Sun, Sep 16, 2012 at 10:00:38PM +0200, Markus Mohrhard wrote:
> Hey guys,
>
> I was debugging a calc crash for the last days that seems to be
> related to ScRangeList::UpdateReference ([1]). As Kohei found out
> removing
>
>     // delete all entries that are fully deleted
>     if( eUpdateRefMode == URM_INSDEL && (nDx < 0 || nDy < 0) )
>     {
>         vector<ScRange*>::iterator itr =
> std::remove_if(maRanges.begin(), maRanges.end(), FindDeletedRange(nDx,
> nDy));
>         for_each(itr, maRanges.end(), ScDeleteObjectByPtr<ScRange>());
>         maRanges.erase(itr, maRanges.end());
>     }
>
> from the method prevents this crash. At least according to my
> understanding of c++ this piece of code is at least from a c++ POV
> correct (from a calc POV it is wrong).

The problem is that remove_if (and remove too) only copies the "good"
elements to the left, but it does not care what remains in the rest of
the range. As an example (in pseudo-C++), after:

v = {1, 2, 3, 4, 5};
remove(v.begin(), v.end(), 3);

v might look like {1, 2, 4, 5, 5}.

IOW, there is no guarantee that the tail of the vector will contain the
"removed" elements. And most probably it will not.

You can use copy_if as an additional step before remove_if to get the
deleted ranges and then use for_each on that vector. But I guess
explicit loop might be simpler and easier to understand in this case.

D.