Hi Markus,
On Sun, Aug 5, 2012 at 8:29 AM, Markus Mohrhard
<
[hidden email]> wrote:
> [1] is mostly a fix for a strange behavior of ScRangeList which
> affects also conditional formats. ScRangeList did not delete a ScRange
> if UpdateReference removed all the ScRange cells. The second step of
> the patch is to remove the conditional formatting if the ScRangeList
> is empty after updating the range.
...
> [1]
http://cgit.freedesktop.org/libreoffice/core/commit/?id=76f56b5e8d4abf17682aa75b7cf183b883809234Regarding
iterator itr = begin();
while(itr != end())
{
if(itr->GetRange().empty())
maConditionalFormats.erase(itr++);
else
++itr;
}
that erase line causes an undefined behavior, and subtle, hard-to-find
bug later. maConditionalFormats is a boost::ptr_set which uses
std::set in its implementation. The std::set's erase call invalidates
the iterator of the element that has just been deleted, and that line
increments the iterator after it's been invalidated. Instead of doing
post-increment on the invalidated iterator, doing it this way
iterator itr = begin();
while(itr != end())
{
if(itr->GetRange().empty())
{
iterator itErase = itr;
++itr;
maConditionalFormats.erase(itErase);
}
else
++itr;
}
_should_ work.
c.f.
http://stackoverflow.com/questions/1636578/iterator-validity-after-erase-call-in-stdsetAlso, this line
class FindDeletedRange : public ::std::unary_function<bool, const ScRange*>
should be
class FindDeletedRange : public ::std::unary_function<const ScRange*,
bool> (swap the 1st and 2nd template arguments.)
The rest looks good to me.
Kohei
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Locked
