[Libreoffice] [REVIEW] fdo#38457: Crash in DataPilot on moving the fields

Ladies & gents, 

attached patch (somehow) fixes the crash in Calc's DataPilot caused by field 
indexes being off-by-N. Consequent attempt to move such fields leads to 
segfault.

It is regression from 3.3, the orig code was as follows:

    SCCOL nColAdd = 0;
    if ( bForFile )
    {
        // in old file format, columns are within document, not within source 
range

        DBG_ASSERT( pSheetDesc, "FillOldParam: bForFile, !pSheetDesc" );
        nColAdd = pSheetDesc->aSourceRange.aStart.Col();
    }

where 'if' branch has been unused since 2004 at least, as 'bForFile' variable 
has been always set to false elsewhere in the code.

Later, it has been changed into this: 

SCCOL nColAdd = pSheetDesc->GetSourceRange().aStart.Col();

apparently to fix a build failure (commit 523a8f41388f6d). But this seems to 
produce wrong results -- shifts column index of DP field by N and attempt to 
move such field crashes the whole thing. 

From the comment in the 3.3 code (mentioning "old file format") I guess we can 
assume column offset always set to 0 i.e. no column offset at all. But not 
really sure about that, this is Yoshida-san's baby.  

Consequently, some further code cleanup could be needed (i.e. nCollAdd param 
can be completely removed from lcl_FillOldFields() ), but that's more a 
material for master branch

hB.
-- 
  \\\\\              Katarina Machalkova    
  \\\\\\\__o          LibO developer
__\\\\\\\'/_          & hedgehog painter

From 19dada4d1cf45b1042788cd6ec3c7ea56c45e3a6 Mon Sep 17 00:00:00 2001
From: Katarina Machalkova <kmachalkova@suse.cz>
Date: Mon, 27 Jun 2011 13:35:01 +0200
Subject: [PATCH] fdo#38456: Always set column offset to 0

Setting it to anything else moves field indexes off-by-N and causes crash
in some cases (the original code has been unused since 2004 at least)
---
 sc/source/core/data/dpobject.cxx |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/sc/source/core/data/dpobject.cxx b/sc/source/core/data/dpobject.cxx
index 472c451..93bfebb 100644
--- a/sc/source/core/data/dpobject.cxx
+++ b/sc/source/core/data/dpobject.cxx
@@ -1860,9 +1860,6 @@ sal_Bool ScDPObject::FillOldParam(ScPivotParam& rParam) const
     // ppLabelArr / nLabels is not changed
 
     SCCOL nSrcColOffset = 0;
-    if (IsSheetData())
-        // source data column offset is only for internal sheet source.
-        nSrcColOffset = pSheetDesc->GetSourceRange().aStart.Col();
 
     bool bAddData = ( lcl_GetDataGetOrientation( xSource ) == 
sheet::DataPilotFieldOrientation_HIDDEN );
     lcl_FillOldFields(
-- 
1.7.3.4

Attachment: signature.asc
Description: This is a digitally signed message part.