Re: Invalid iterator in RelationTableView.cxx? (dbaccess module)

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