Re: Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-03 Thread Stephan Bergmann
On 02/02/2012 09:08 PM, julien2412 wrote: Would this patch better ? (I kept the for loop) Unfortunately that still has a problem. After rBoxes.erase(toErase), it (which is the same as toErase) is invalidated, so incrementing it (up in the for(...;...;...) part) has undefined behavior. The

Re: Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-03 Thread Michael Stahl
On 03/02/12 14:01, Stephan Bergmann wrote: On 02/02/2012 09:08 PM, julien2412 wrote: Would this patch better ? (I kept the for loop) Unfortunately that still has a problem. After rBoxes.erase(toErase), it (which is the same as toErase) is invalidated, so incrementing it (up in the

Re: Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-03 Thread Stephan Bergmann
On 02/03/2012 04:18 PM, Michael Stahl wrote: On 03/02/12 14:01, Stephan Bergmann wrote: The standard idiom is for (iterator i = m.begin(); i != m.end();) { if (doErase) { m.erase(i++); } else { ++i; } } but doesn't that have the same problem? i is

Re: Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-03 Thread Michael Stahl
On 03/02/12 17:21, Stephan Bergmann wrote: On 02/03/2012 04:18 PM, Michael Stahl wrote: On 03/02/12 14:01, Stephan Bergmann wrote: The standard idiom is for (iterator i = m.begin(); i != m.end();) { if (doErase) { m.erase(i++); } else { ++i; } }

Re: Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-03 Thread Stephan Bergmann
On 02/03/2012 05:31 PM, Michael Stahl wrote: ah, that's surprising. that's shocking ;) see, that is why i almost always write the i++ as an extra statement, i'm never quite exactly sure what it does, and when :) There is a sequence point (in C++03 parlance; the nomenclature changed

Re: Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-03 Thread Terrence Enger
On Fri, 2012-02-03 at 17:31 +0100, Michael Stahl wrote: On 03/02/12 17:21, Stephan Bergmann wrote: On 02/03/2012 04:18 PM, Michael Stahl wrote: On 03/02/12 14:01, Stephan Bergmann wrote: The standard idiom is for (iterator i = m.begin(); i != m.end();) { if (doErase) {

Re: Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-03 Thread Michael Stahl
On 03/02/12 18:17, Terrence Enger wrote: Stephan, I am sorry to question your expertise, but I wonder ... is your reassurance based on knowledge of the language standard, or is based on observed behaviour of C++ compilers? hahaha, i believe that among all the developers who ever worked on

Re: Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-03 Thread Terrence Enger
On Fri, 2012-02-03 at 18:30 +0100, Michael Stahl wrote: On 03/02/12 18:17, Terrence Enger wrote: Stephan, I am sorry to question your expertise, but I wonder ... is your reassurance based on knowledge of the language standard, or is based on observed behaviour of C++ compilers? hahaha,

Re: Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-03 Thread julien2412
I followed the idiom you indicated, commited and pushed on master (see 9d0136679e441413b6945d2a40aa892b50ee19a8) Thank you Stephan for the details about C++ Hope that C++11 will spread quickly because all these things aren't very intuitive. Julien -- View this message in context:

Re: [Libreoffice] Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-02 Thread Stephan Bergmann
On 02/01/2012 11:40 PM, julien2412 wrote: Cppcheck reports this : core/sw/source/core/fields/cellfml.cxx 970 StlMissingComparisonstyle Missing bounds check for extra iterator increment in loop. Here are the lines : 961 // dann mal die Tabellenkoepfe raus: 962

Re: [Libreoffice] Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-02 Thread Marcel Metz
Hello Julien On 02/01/2012 11:40 PM, julien2412 wrote: Here are the lines : 961 // dann mal die Tabellenkoepfe raus: 962 for( SwSelBoxes::iterator it = rBoxes.begin(); it != rBoxes.end(); ++it ) 963 { 964 pLine =

Re: [Libreoffice] Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-02 Thread Lubos Lunak
On Thursday 02 of February 2012, Stephan Bergmann wrote: On 02/01/2012 11:40 PM, julien2412 wrote: Here are the lines : 961 // dann mal die Tabellenkoepfe raus: 962 for( SwSelBoxes::iterator it = rBoxes.begin(); it != rBoxes.end(); ++it ) 963

Re: [Libreoffice] Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-02 Thread Stephan Bergmann
On 02/02/2012 10:13 AM, Marcel Metz wrote: If the box that is represented by `it` should be deleted you could use. 970 it = rBoxes.erase( it ); Unfortunately, this is only C++11, not C++03. Stephan ___ LibreOffice mailing list

Re: [Libreoffice] Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-02 Thread Stephan Bergmann
On 02/02/2012 11:26 AM, Lubos Lunak wrote: I agree with all the points, but in Julien's defense, I remember exactly this same approach was pushed in recently as a fix to the same issue elsewhere. Might well be, I probably didn't notice. And this is in no way meant to criticize Julien --

Re: [Libreoffice] Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-02 Thread Lubos Lunak
On Thursday 02 of February 2012, Stephan Bergmann wrote: On 02/02/2012 11:26 AM, Lubos Lunak wrote: I agree with all the points, but in Julien's defense, I remember exactly this same approach was pushed in recently as a fix to the same issue elsewhere. Might well be, I probably didn't

Re: [Libreoffice] Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-02 Thread Stephan Bergmann
On 02/02/2012 02:06 PM, Lubos Lunak wrote: I definitely didn't mean to get formal here, I simply meant a description of how to actually handle this, as it turns up now and then, and STL doesn't make this trivial. In which case the answer should be to read Item 9 Choose carefully among

Re: Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-02 Thread julien2412
Would this patch better ? (I kept the for loop) diff --git a/sw/source/core/fields/cellfml.cxx b/sw/source/core/fields/cellfml.cxx index 33b953e..5c626dd 100644 --- a/sw/source/core/fields/cellfml.cxx +++ b/sw/source/core/fields/cellfml.cxx @@ -967,8 +967,8 @@ void SwTableFormula::GetBoxes( const

[Libreoffice] Question about iterator management in sw/source/core/fields/cellfml.cxx

2012-02-01 Thread julien2412
Hi, Cppcheck reports this : core/sw/source/core/fields/cellfml.cxx 970 StlMissingComparisonstyle Missing bounds check for extra iterator increment in loop. Here are the lines : 961 // dann mal die Tabellenkoepfe raus: 962 for( SwSelBoxes::iterator it =