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

On Wed, 2011-06-29 at 22:24 +0200, Markus Mohrhard wrote:
> Kohei, I think, as this is mostly your area, you might want to have a
> closer look at the whole code around the offset problem.

So, everything comes down to what PivotField::nCol is expected to
contain; is it a (always 0-based) dimension index, or a column index
whose smallest index may not be always 0.  The column offset variable
was there because PivotField::nCol was expected to contain a column
index rather than a dimension index if the data source was a sheet,
while for the other data source types it was expected to contain a
dimension index.

First things first, Bubli's fix appears to work just fine.  This is all
good.

Now, that would also imply that in the latest pivot table code,
PivotField::nCol always contain a dimension index even for the internal
sheet data source type.  I remember I fixed some bugs some time ago in
this area because nCol didn't contain a column index, so I know for a
fact that this was not always the case.

Maybe my re-work of the dialog changed this, my cache table re-work did
this.  I don't know for sure.  But I have to keep this fact in mind
going forward, in case I come across any old code that may still expect
PivotField::nCol to contain a column index.  But that's fine since I
personally prefer it always containing a dimension index anyway. :-)

Regards,

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc
<kyoshida@novell.com>