Here is another bug fix I'd like to push to the -3-4 branch.
https://bugs.freedesktop.org/show_bug.cgi?id=36719
Opening a document with a pivot table that is linked to a invalid
database source crashes Calc in various places. The attached patch
fixes this.
The changes are mostly simple things such as checking for NULL pointers
before dereferencing.
I'm attaching a patch instead of cherry-picking because the commit I
pushed to master has a wrong bug ID in the message.
Anyway, review & sign-off appreciated.
Kohei
--
Kohei Yoshida, LibreOffice hacker, Calc
<kyosh...@novell.com>
>From 9b4370d6946f9dee5d363e362d86247df7d2958c Mon Sep 17 00:00:00 2001
From: Kohei Yoshida <kyosh...@novell.com>
Date: Fri, 10 Jun 2011 13:06:49 -0400
Subject: [PATCH] fdo#36719: Prevent crash on pivot table with invalid database connection.
Check for NULL cache state to handle invalid database connection more
gracefully (i.e. without crashing).
---
sc/source/core/data/dpcachetable.cxx | 11 ++++++++---
sc/source/core/data/dpsdbtab.cxx | 10 +++++++++-
sc/source/ui/view/cellsh1.cxx | 24 +++++++++++++++++++++++-
3 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/sc/source/core/data/dpcachetable.cxx b/sc/source/core/data/dpcachetable.cxx
index 40d34ac..b7b8a64 100644
--- a/sc/source/core/data/dpcachetable.cxx
+++ b/sc/source/core/data/dpcachetable.cxx
@@ -180,12 +180,12 @@ ScDPCacheTable::~ScDPCacheTable()
sal_Int32 ScDPCacheTable::getRowSize() const
{
- return getCache()->GetRowCount();
+ return mpCache ? getCache()->GetRowCount() : 0;
}
sal_Int32 ScDPCacheTable::getColSize() const
{
- return getCache()->GetColumnCount();
+ return mpCache ? getCache()->GetColumnCount() : 0;
}
void ScDPCacheTable::fillTable(
@@ -313,6 +313,9 @@ void ScDPCacheTable::filterByPageDimension(const vector<Criterion>& rCriteria, c
const ScDPItemData* ScDPCacheTable::getCell(SCCOL nCol, SCROW nRow, bool bRepeatIfEmpty) const
{
+ if (!mpCache)
+ return NULL;
+
SCROW nId= getCache()->GetItemDataId(nCol, nRow, bRepeatIfEmpty);
return getCache()->GetItemDataById( nCol, nId );
}
@@ -331,6 +334,8 @@ void ScDPCacheTable::getValue( ScDPValueData& rVal, SCCOL nCol, SCROW nRow, boo
}
String ScDPCacheTable::getFieldName(SCCOL nIndex) const
{
+ if (!mpCache)
+ return String();
return getCache()->GetDimensionName( nIndex );
}
@@ -410,7 +415,7 @@ void ScDPCacheTable::filterTable(const vector<Criterion>& rCriteria, Sequence< S
SCROW ScDPCacheTable::getOrder(long nDim, SCROW nIndex) const
{
- return getCache()->GetOrder(nDim, nIndex);
+ return mpCache ? getCache()->GetOrder(nDim, nIndex) : 0;
}
void ScDPCacheTable::clear()
diff --git a/sc/source/core/data/dpsdbtab.cxx b/sc/source/core/data/dpsdbtab.cxx
index 417c142..d16d03e 100644
--- a/sc/source/core/data/dpsdbtab.cxx
+++ b/sc/source/core/data/dpsdbtab.cxx
@@ -136,10 +136,18 @@ void ScDatabaseDPData::SetEmptyFlags( sal_Bool /* bIgnoreEmptyRows */, sal_Bool
void ScDatabaseDPData::CreateCacheTable()
{
if (!aCacheTable.empty())
+ // cache table already created.
return;
if (!aCacheTable.hasCache())
- aCacheTable.setCache(mrImport.CreateCache());
+ {
+ const ScDPCache* pCache = mrImport.CreateCache();
+ if (!pCache)
+ // Cache creation failed. Perhaps invalid database connection.
+ return;
+
+ aCacheTable.setCache(pCache);
+ }
aCacheTable.fillTable();
}
diff --git a/sc/source/ui/view/cellsh1.cxx b/sc/source/ui/view/cellsh1.cxx
index 30d3933..90fb0ba 100644
--- a/sc/source/ui/view/cellsh1.cxx
+++ b/sc/source/ui/view/cellsh1.cxx
@@ -2127,6 +2127,27 @@ void ScCellShell::ExecuteExternalSource(
_rRequest.Ignore();
}
+namespace {
+
+bool isDPSourceValid(const ScDPObject& rDPObj)
+{
+ if (rDPObj.IsImportData())
+ {
+ // If the data type is database, check if the database is still valid.
+ const ScImportSourceDesc* pDesc = rDPObj.GetImportSourceDesc();
+ if (!pDesc)
+ return false;
+
+ const ScDPCache* pCache = pDesc->CreateCache();
+ if (!pCache)
+ // cashe creation failed, probably due to invalid connection.
+ return false;
+ }
+ return true;
+}
+
+}
+
void ScCellShell::ExecuteDataPilotDialog()
{
ScModule* pScMod = SC_MOD();
@@ -2142,7 +2163,8 @@ void ScCellShell::ExecuteDataPilotDialog()
pData->GetTabNo() );
if ( pDPObj ) // on an existing table?
{
- pNewDPObject.reset(new ScDPObject(*pDPObj));
+ if (isDPSourceValid(*pDPObj))
+ pNewDPObject.reset(new ScDPObject(*pDPObj));
}
else // create new table
{
--
1.7.3.4
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
