On 27/08/2013 09:04, Jan Holesovsky wrote:
289 void OHierarchyElement_Impl::RemoveElement( const ::rtl::Reference<
OHierarchyElement_Impl>& aRef )
290 {
291 {
292 ::osl::MutexGuard aGuard( m_aMutex );
293 OHierarchyElementList_Impl::iterator aIter =
m_aChildren.begin();
294 const OHierarchyElementList_Impl::const_iterator aEnd =
m_aChildren.end();
295 while (aIter != aEnd)
296 {
297 if (aIter->second == aRef )
298 aIter = m_aChildren.erase(aIter);
299 else
300 ++aIter;
301 }
302 }
See
http://opengrok.libreoffice.org/xref/core/package/source/xstor/ohierarchyholder.cxx#298
Is it ok to use "aEnd" or, since erase may be called, we should change the
while into:
while (aIter != m_aChildren.end())
(and remove aEnd)
In this exact case (when the value may be present more times in the
vector), you might want to use the Erase-remove idiom [1]:
// remove all occurrences of aRef
m_aChildren.erase(std::remove(m_aChildren.begin(), m_aChildren.end(), aRef),
m_aChildren.end());
Even with the comment I suppose, so that people who haven't read tons of
C++ books can see what's going on ;-)
I copy pasted this exact line and it failed to compile :-(
Another thing: couldn't we break the loop after erase or could aRef be
present several times?
Not sure - worth checking the history of that file I think.
According to git history, it's been like this since first git log, so
let's let this.
About end iterator, it was my fault since I had created a const end
iterator in the beginning of this year. I've fixed this (see
http://cgit.freedesktop.org/libreoffice/core/commit/?id=9ebcd5ec54ec5d77cf46849f7f00bf915644f6e1)
Julien
_______________________________________________
LibreOffice mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
