[Libreoffice] Posable bug

[Joseph Powers]

File: sot/source/sdstor/ucbstorage.cxx

Code in question:


BOOL UCBStorage_Impl::Revert()
{
    UCBStorageElement_Impl* pElement = m_aChildrenList.First();
    while ( pElement )
    {
        pElement->m_bIsRemoved = FALSE;
        if ( pElement->m_bIsInserted )
        {
            m_aChildrenList.Remove( pElement );  // correct usage of list ???
        }
        else
        {
            if ( pElement->m_xStream.Is() )
            {
                pElement->m_xStream->m_bCommited = sal_False;
                pElement->m_xStream->Revert();
            }
            else if ( pElement->m_xStorage.Is() )
            {
                pElement->m_xStorage->m_bCommited = sal_False;
                pElement->m_xStorage->Revert();
            }

            pElement->m_aName = pElement->m_aOriginalName;
            pElement->m_bIsRemoved = FALSE;
        }
        pElement = m_aChildrenList.Next();
    }
    return TRUE;
}

This what I'm thinking:
1) The Remove() will delete the current element and basically move the pointer to the next item.
2) The Next() move the pointer to the next item.
3) Thus, if an item is removed we'll skip the next item in the list.

I'm converting this to a vector<> to it's easy to fix:
for ( i = 0, i < size(); ) {
  if () {
	Remove()
  } else {
	++i
  }
}

Question: Am I reading this correctly?

I need to go to work now; so I'll check back later to see if anyone knows a reason why we should be skipping the element after the removed one. If no one complains I'll just fix it tonight after work.

Joe P.

PS: The "correct usage of list???" comment is someone else's.