Julien, Looks to me like a very nice catch ...
Here is the version as of 2011.06.15 showing braces on the for loops in both then and else blocks... from open grok -- http://opengrok.libreoffice.org/xref/core/sc/source/ui/docshell/docfunc.cxx?r=1b363f632110e80ead67ff376e92e4487556ca55 3735 3736 if (bSize) 3737 { 3738 SCCOLROW nCols[2] = { nStartCol, nEndCol }; 3739 SCCOLROW nRows[2] = { nStartRow, nEndRow }; 3740 3741 for (SCTAB nTab=0; nTab<nTabCount; nTab++) 3742 if (aMark.GetTableSelect(nTab)) 3743 { 3744 SetWidthOrHeight( sal_True, 1,nCols, nTab, SC_SIZE_VISOPT, STD_EXTRA_WIDTH, false, sal_True); 3745 SetWidthOrHeight( false,1,nRows, nTab, SC_SIZE_VISOPT, 0, false, false); 3746 rDocShell.PostPaint( 0,0,nTab, MAXCOL,MAXROW,nTab, 3747 PAINT_GRID | PAINT_LEFT | PAINT_TOP ); 3748 } 3749 } 3750 else 3751 { 3752 for (SCTAB nTab=0; nTab<nTabCount; nTab++) 3753 if (aMark.GetTableSelect(nTab)) 3754 { 3755 sal_Bool bAdj = AdjustRowHeight( ScRange(nStartCol, nStartRow, nTab, 3756 nEndCol, nEndRow, nTab), false ); 3757 if (bAdj) 3758 rDocShell.PostPaint( 0,nStartRow,nTab, MAXCOL,MAXROW,nTab, 3759 PAINT_GRID | PAINT_LEFT ); 3760 else 3761 rDocShell.PostPaint( nStartCol, nStartRow, nTab, 3762 nEndCol, nEndRow, nTab, PAINT_GRID ); 3763 } 3764 } The next version (2011.07.05) http://opengrok.libreoffice.org/xref/core/sc/source/ui/docshell/docfunc.cxx?r=2b88f6d32f572792597ccbb15276b9db52db7d10 is a commit labelled "change remaining manual loops to ScMarkData::iterator " and the braces disappear on just one of the loops. Because the braces were once there and, especially because the else block has ~ same action within the braces, I say that they should definitely come back. Even though, I am not at all clear what this code is doing (whilst autoformatting selected cells in Calc)... this is an obvious oopsie within a greater code cleanup that should be corrected. This is not fdo# 47466 -- that crash gets close but doesn't quite get to this code: that non-reproducible crash occurred inside the AutoFormat function called just above the missing braces. After some more looking at the code, it appears to be applying widths and heights... Some testing shows that if one selects a tabular area and apply a non-default width and height (Format->Rows/Columns->Height/Width) then and uncheck AutoFormat Width and Height under AutoFormat... -> More. Doing the AutoFormat then forgets the width (doesn't loop) but not the height (still loops). The block of code in question starts to look like maybe it is fix code that corrects an overactive AutoFormat function called just above. This here bug -- https://bugs.freedesktop.org/show_bug.cgi?id=51524 Shows similar loss-of-width behavior in collapse/uncollapse (as opposed to straight up AutoFormat) There are also several reports of loss of height information (including self-healing upon edit) - most of them are closed &/or related to file open/close. Maybe this block of W/H fix code is missing elsewhere, maybe it is not needed here anymore, maybe the open & save fix the values better than this now, maybe this simply is the width/height portion of the AutoFormat ... hmmmm I will check to see if inserting braces in the code fixes whatever loss-of-width/height behavior that I can reproduce or find. My 2cents... and now a couple more: I note that, for all its type mushiness, BASIC will not allow this kind of mistake. The then-clause must either be all on one line (can do IF test THEN StmtA : StmtB : StmtC) and a multi-line then-clause must be terminated with End If or Else. Julien, I think you should get the patch credit - this is your find. However, if it will help you continue to run cppchecks and find more of these interesting puzzles, I would be happy to patch a pair of braces for you and mark the appropriate bugs as resolved by you ;-D Sweet catch, LeMoyne -- View this message in context: http://nabble.documentfoundation.org/Wrong-indentation-which-leads-to-segfault-in-sc-source-ui-docshell-docfunc-cxx-tp4026726p4026751.html Sent from the Dev mailing list archive at Nabble.com. _______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
