From: Michal H. <ms...@gm...> - 2008-01-23 13:41:22
|
On Tue, Jan 22, 2008 at 10:50:36PM +0000, Jozef Misutka wrote: > you stated that correctly, only iterator i gets invalidated after erase BUT i++ or ++i is calling a function on an invalidated iterator. what happens? I have looked into STL code and it seems that erase method invalidates only data of the tree node represented by by given iterator and links in tree are not changed (please look at stl_tree.h and _Rb_tree::erase called from map::erase and _M_destroy_node function which is called used for real work). > can you call functions on invalidated iterators? I think you can't dereference them. Note that some containers invalidates all iterators when inserting or removing (you can't even {inc/dec}ment them). Map keeps those iterators after both operations. > i am not sure about the technical skills of the guys discussing this topic but i think you can get the point herehttp://www.velocityreviews.com/forums/t283023-stl-stdmap-erase.html it is good to know what happens exactly, but when the only person who knows is YOU then other developers can get confused. so better use 100% correct code (at least in pdfedit :))... jozo If you see some flaw in my thinking, please point them out. I will change the code accordingly. Otherwise I will keep current one, because we have no crashes in that area and complicating loop and iterator logic is not worthy IMO. > > > Date: Tue, 22 Jan 2008 13:57:55 +0100From: ms...@gm...To: pdf...@li...Subject: Re: [Pdfedit-devel] stl iterators > On Jan 22, 2008 12:13 AM, Jozef Misutka <mis...@ho...> wrote: > > there are several places where std::erase+iterators are not used correctly > > please correct it you can use the provided patch jozo Index: cpdf.cc===================================================================RCS file: /cvsroot/pdfedit/pdfedit/src/kernel/cpdf.cc,vretrieving revision 1.86.2.1 diff -u -r1.86.2.1 cpdf.cc--- cpdf.cc 18 Jan 2008 00:46:05 -0000 1.86.2.1+++ cpdf.cc 21 Jan 2008 22:45:22 -0000@@ -2401,7 +2401,7 @@ IndiRef ref=getValueFromSimple<CRef>(oldValue); bool found=false;- for(PageList::iterator i=pageList.begin(); i!=pageList.end(); ++i)+ for(PageList::iterator i=pageList.begin(); i!=pageList.end();) { shared_ptr<CPage> page=i->second; // checks page's dictionary whether it is in oldDict_ptr sub@@ -2419,16 +2419,19 @@ page->invalidate(); // NOTE: std::map keeps iterators following iterators // valid after removing - pageList.erase(i);+ pageList.erase(i++); > This is not an error or bad usage of map iterators AFAIK. Comment above eraseis little bit confusing, but it states that iterators behind (and before too) i are *not* invalided by erase method. This means that I can do i++ (i-- etc.) unless I do *dereference* this iterator. From (http://www.sgi.com/tech/stl/Map.html):" Erasing an element from a map also does not invalidate any iterators, except, of course, for iterators that actually point to the element that is being erased."But maybe I have misunderstood something. I will go over my code again and check this again[...]-- Michal Hocko -- Michal Hocko |