Invalid iterator in RelationTableView.cxx? (dbaccess module)

[Lionel Elie Mamane]

On Tue, Feb 10, 2015 at 12:02:55PM +0100, Michael Stahl wrote:
> On 10.02.2015 10:03, Lionel Elie Mamane wrote:
> > On Mon, Feb 09, 2015 at 11:00:10PM +0100, Michael Stahl wrote:
> >> On 09.02.2015 22:22, julien2412 wrote:
> > 
> >>> On RelationTableView.cxx file, I noticed this:
> >>>      99     for(;aIter != rTabWinDataList.rend();++aIter)
> >>>     100     {
> >>>     101         TTableWindowData::value_type pData = *aIter;
> >>>     102         OTableWindow* pTabWin = createWindow(pData);
> >>>     103 
> >>>     104         if (!pTabWin->Init())
> >>>     105         {
> >>> ... 
> >>>     112             rTabWinDataList.erase(
> >>> ::std::remove(rTabWinDataList.begin(), rTabWinDataList.end(), *aIter),
> >>> rTabWinDataList.end());
> >>>     113             continue;
> >>>     114         }
> > 
> >>> Shouldn't the "continue" be replaced by "break" to avoid invalid iterator or
> >>> "aIter" isn't invalid for "for loops" at all even after "erase" lines?
> > 
> > 
> >> ... so since the element that aIter points to will be deleted, aIter
> >> is invalid.

>>> hmm.... perhaps you could replace the erase(remove...) with an erase
>>> that takes the reverse_iterator's base iterator (aIter.base()) to
>>> erase just one element

>> I'm not convinced the erase is supposed to delete only one element,
>> unless rTabWinDataList has no duplicate. But if it has no duplicate,

> if there are duplicates then the current code is not just subtly but
> obviously wrong, since on remove the aIter needs to be incremented not
> just once but once for every removed duplicate.

I don't know why I thought that there would be at most
(rbegin()-aIter+1) duplicates, in which case the std::remove has moved
all duplicates at or after aIter. There is no reason for my premise to
hold.

>> then maybe we can simplify all this complicated remove/erase
>> construction and just do:

>> only once on each position/value, as opposed to possibly several times
>> now... Is this "several times the same value" actually desired or a
>> bug? Only understanding the intent of the code will tell.

> well it's probably a bug, if creating the thingy worked the first
> time there's no point in trying again?

Makes sense.

-- 
Lionel