Thanks for your answer. Darshan pointed out the problem on IRC, and I just
removed the erase from the loop and put a clear after it. The iterator being
a member class is just for code concision, as the declaration for an
iiterator is so insanely long, I didn't want to do one each time I add to
iterate over m_tabs :)
2011/1/7 Duane Griffin <dua...@dghda.com>
> On 7 January 2011 10:20, Raphael Langella <raphael.lange...@gmail.com>
> wrote:
> > for (m_tabs_it = m_tabs.begin(); m_tabs_it != m_tabs.end();
> ++m_tabs_it)
> >
> > m_tabs is a std::map. It works fine when it's empty, but as soon as there
> is
> > an element, gdb crashes at the beginning of the second iteration, when it
> > tries to increment the iterator (m_tabs_it). Here is the error:
>
> That line looks fine as far as it goes, but:
>
> for (m_tabs_it = m_tabs.begin(); m_tabs_it != m_tabs.end(); ++m_tabs_it)
> {
> delete (*m_tabs_it).second;
> m_tabs.erase(m_tabs_it);
> m_layers[LAYER_NORMAL].m_regions.pop_back();
> }
>
> ...after m_tabs.erase(m_tabs_it) the iterator is invalidated and can't
> be incremented to get to the next element. As it stands you are
> accessing deleted memory. You need something like this:
>
> m_tabs_it = m_tabs.begin();
> std::map<int, TabbedRegion*>::iterator next;
> while (m_tabs_it != m_tabs.end())
> {
> next = m_tabs_it++;
> delete m_tabs_it->second;
> m_tabs.erase(m_tabs_it);
> m_layers[LAYER_NORMAL].m_regions.pop_back();
> m_tabs_it = next;
> }
>
> However, since you are going through and deleting everything anyway,
> you may want to do something more STL-stylie, eg:
>
> struct ClearTabs {
> public:
> ClearTabs(std::vector<Region*> ®ions): m_regions(regions) {}
> void operator()(std::map<int, TabbedRegion*>::value_type &value)
> {
> delete value.second;
> m_regions.pop_back();
> }
>
> private:
> std::vector<Region*> &m_regions;
> };
>
> bool TilesFramework::layout_statcol(bool show_gold_turns)
> {
> std::for_each(m_tabs.begin(), m_tabs.end(),
> ClearTabs(m_layers[LAYER_NORMAL].m_regions));
> m_tabs.clear();
> ....
>
> You could probably simplify that significantly if there are no subtle
> interactions between the regions and the tabs (i.e. do the deletion
> and popping separately).
>
> Also, from a very quick look there doesn't seem to be any good reason
> for m_tabs_it to be a member variable. It should probably be replaced
> by local variables everywhere it is used in the class. Apologies if
> I've missed something subtle.
>
> Cheers,
> Duane.
>
> --
> "I never could learn to drink that blood and call it wine" - Bob Dylan
>
>
> ------------------------------------------------------------------------------
> Gaining the trust of online customers is vital for the success of any
> company
> that requires sensitive data to be transmitted over the Web. Learn how to
> best implement a security strategy that keeps consumers' information secure
> and instills the confidence they need to proceed with transactions.
> http://p.sf.net/sfu/oracle-sfdevnl
> _______________________________________________
> Crawl-ref-discuss mailing list
> Crawl-ref-discuss@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/crawl-ref-discuss
>
------------------------------------------------------------------------------
Gaining the trust of online customers is vital for the success of any company
that requires sensitive data to be transmitted over the Web. Learn how to
best implement a security strategy that keeps consumers' information secure
and instills the confidence they need to proceed with transactions.
http://p.sf.net/sfu/oracle-sfdevnl
_______________________________________________
Crawl-ref-discuss mailing list
Crawl-ref-discuss@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/crawl-ref-discuss
