[Libreoffice] [PATCH] ScGlobal::GetAutoFormat not always required to create fresh instance

The number one reason why "make check" fails for me is a SEGV in 
soffice.bin during termination of JunitTest_sc_unoapi, namely at

> #0  0x00007f4906ecc0d2 in ScGlobal::GetRscString (nIndex=71) at sc/source/core/data/global.cxx:355
> #1  0x00007f4906f5e968 in ScAutoFormat::ScAutoFormat (this=0x335c510, nLim=4, nDel=4, bDup=0 
> '\000') at sc/source/core/tool/autoform.cxx:898
> #2  0x00007f4906ecbe93 in ScGlobal::GetAutoFormat () at sc/source/core/data/global.cxx:304
> #3  0x00007f490749b799 in ScAutoFormatObj::~ScAutoFormatObj (this=0x7f48fd089a10) at 
> sc/source/ui/unoobj/afmtuno.cxx:441
> #4  0x00007f490749b880 in ScAutoFormatObj::~ScAutoFormatObj (this=0x7f48fd089a10) at 
> sc/source/ui/unoobj/afmtuno.cxx:447
> #5  0x00007f492e87acd1 in cppu::OWeakObject::release (this=0x7f48fd089a10) at 
> cppuhelper/source/weak.cxx:213
> [...]
> #40 0x00007f491736fdcc in binaryurp::Bridge::terminate (this=0x7f491538f1d0) at 
> binaryurp/source/bridge.cxx:271
> [...]
> #55 0x00007f492f7d2377 in desktop::Desktop::DeregisterServices (this=0x7ffff56496c0) at 
> desktop/source/app/appinit.cxx:378
> #56 0x00007f492f7ba4ee in desktop::Desktop::doShutdown (this=0x7ffff56496c0) at 
> desktop/source/app/app.cxx:1875
> #57 0x00007f492f7b9a9d in desktop::Desktop::Main (this=0x7ffff56496c0) at 
> desktop/source/app/app.cxx:1845
> #58 0x00007f492af3c6de in ImplSVMain () at vcl/source/app/svmain.cxx:178
> #59 0x00007f492af3c824 in SVMain () at vcl/source/app/svmain.cxx:215
> #60 0x00007f492f7f438a in soffice_main () at desktop/source/app/sofficemain.cxx:67
> #61 0x0000000000400734 in sal_main () at desktop/source/app/main.c:34
> #62 0x0000000000400719 in main (argc=9, argv=0x7ffff5649878) at desktop/source/app/main.c:33

ScGlobal::ppRscString has apparently already been cleared at this point. 
 But looking deeper down the stack, there is no real reason to create a 
fresh ScAutoFormat at all -- ~ScAutoFormatObj does

  ScAutoFormat* pFormats = ScGlobal::GetAutoFormat();
  if ( pFormats && pFormats->IsSaveLater() )

but the freshly created ScAutoFormat (from ScGlobal::GetAutoFormat) has 
bSaveLater set to false, so the condition would correctly already 
evaluate to false if ScGlobal::GetAutoFormat would return null here.  It 
is also curious that pFormats is checked for null here, even though 
ScGlobal::GetAutoFormat never returns null.

With that insight, I inspected all the other calls to 
ScGlobal::GetAutoFormat, and many (but not all) of them too would only 
dereference the returned ScAutoFormat pointer after checking it for 
non-nullness.  So I came up with the attached patch, introducing 
ScGlobal::GetOrCreateAutoFormat for the few cases that apparently 
require a non-null ScAutoFormat, and letting ScGlobal::GetAutoFormat 
return null in case ScGlobal::pAutoFormat is null.

However, I had to fix one of the non-null--checking cases to use 
GetOrCreateAutoFormat, as otherwise 
sc/CppunitTest_sc_tableautoformatfield would fail with unexpected 
IndexOutOfBoundsExceptions.

So, I would appreciate it if some Calc aficionado could look at the 
patch, whether it actually makes any sense.  It would be crucial to look 
at the remaining calls to ScGlobal::GetAutoFormat, in

  sc/source/core/data/table4.cxx
  sc/source/ui/docshell/docfunc.cxx
  sc/source/ui/unoobj/afmtuno.cxx
  sc/source/ui/unoobj/cellsuno.cxx

and check whether not creating a fresh ScAutoFormat is OK for them, or 
whether they should be calls to GetOrCreateAutoFormat after all (and the 
confusing null-checks removed).

Stephan

From 9ff9055ae635ac2224adfc6601f27561956a5e74 Mon Sep 17 00:00:00 2001
From: Stephan Bergmann <sbergman@redhat.com>
Date: Fri, 6 Jan 2012 18:55:28 +0100
Subject: [PATCH] ScGlobal::GetAutoFormat not always required to create fresh
 instance.

...at least in ~ScAutoFormatObj it appears unnecessary and can lead to
crashes during Desktop::DeregisterServices (when ScGlobal::ppRscString
is already null and ScAutoFormat ctor calls ScGlobal::GetRscString).

Therefore split GetAutoFormat in two, GetOrCreateAutoFormat for cases
that apparently need a non-null return and GetAutoFormat for those that
are (hopefully) also OK with a null return.
---
 sc/inc/global.hxx                     |    1 +
 sc/source/core/data/global.cxx        |    5 +++++
 sc/source/ui/miscdlgs/scuiautofmt.cxx |    4 ++--
 sc/source/ui/unoobj/afmtuno.cxx       |    3 +--
 sc/source/ui/view/cellsh3.cxx         |    4 ++--
 5 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/sc/inc/global.hxx b/sc/inc/global.hxx
index a810400..ac10094 100644
--- a/sc/inc/global.hxx
+++ b/sc/inc/global.hxx
@@ -562,6 +562,7 @@ public:
     SC_DLLPUBLIC static const SvxSearchItem&    GetSearchItem();
     SC_DLLPUBLIC static void                    SetSearchItem( const SvxSearchItem& rNew );
     SC_DLLPUBLIC static ScAutoFormat*       GetAutoFormat();
+    SC_DLLPUBLIC static ScAutoFormat*       GetOrCreateAutoFormat();
     static void                 ClearAutoFormat(); //BugId 54209
     static FuncCollection*      GetFuncCollection();
     SC_DLLPUBLIC static ScUnoAddInCollection* GetAddInCollection();
diff --git a/sc/source/core/data/global.cxx b/sc/source/core/data/global.cxx
index 6de3782..c43a853 100644
--- a/sc/source/core/data/global.cxx
+++ b/sc/source/core/data/global.cxx
@@ -299,6 +299,11 @@ void ScGlobal::ClearAutoFormat()
 
 ScAutoFormat* ScGlobal::GetAutoFormat()
 {
+    return pAutoFormat;
+}
+
+ScAutoFormat* ScGlobal::GetOrCreateAutoFormat()
+{
     if ( !pAutoFormat )
     {
         pAutoFormat = new ScAutoFormat;
diff --git a/sc/source/ui/miscdlgs/scuiautofmt.cxx b/sc/source/ui/miscdlgs/scuiautofmt.cxx
index d51b420..9466a4b 100644
--- a/sc/source/ui/miscdlgs/scuiautofmt.cxx
+++ b/sc/source/ui/miscdlgs/scuiautofmt.cxx
@@ -194,7 +194,7 @@ IMPL_LINK( ScAutoFormatDlg, CloseHdl, PushButton *, pBtn )
     if ( pBtn == &aBtnOk || pBtn == &aBtnCancel )
     {
         if ( bCoreDataChanged )
-            ScGlobal::GetAutoFormat()->Save();
+            ScGlobal::GetOrCreateAutoFormat()->Save();
 
         EndDialog( (pBtn == &aBtnOk) ? RET_OK : RET_CANCEL );
     }
@@ -206,7 +206,7 @@ IMPL_LINK( ScAutoFormatDlg, CloseHdl, PushButton *, pBtn )
 IMPL_LINK_INLINE_START( ScAutoFormatDlg, DblClkHdl, void *, EMPTYARG )
 {
     if ( bCoreDataChanged )
-        ScGlobal::GetAutoFormat()->Save();
+        ScGlobal::GetOrCreateAutoFormat()->Save();
 
     EndDialog( RET_OK );
     return 0;
diff --git a/sc/source/ui/unoobj/afmtuno.cxx b/sc/source/ui/unoobj/afmtuno.cxx
index f9dc03d..9a28b47 100644
--- a/sc/source/ui/unoobj/afmtuno.cxx
+++ b/sc/source/ui/unoobj/afmtuno.cxx
@@ -221,8 +221,7 @@ uno::Sequence<rtl::OUString> ScAutoFormatsObj::getSupportedServiceNames_Static()
 
 ScAutoFormatObj* ScAutoFormatsObj::GetObjectByIndex_Impl(sal_uInt16 nIndex)
 {
-    ScAutoFormat* pFormats = ScGlobal::GetAutoFormat();
-    if (pFormats && nIndex < pFormats->GetCount())
+    if (nIndex < ScGlobal::GetOrCreateAutoFormat()->GetCount())
         return new ScAutoFormatObj(nIndex);
 
     return NULL;    // falscher Index
diff --git a/sc/source/ui/view/cellsh3.cxx b/sc/source/ui/view/cellsh3.cxx
index 19111c8..a9950c8 100644
--- a/sc/source/ui/view/cellsh3.cxx
+++ b/sc/source/ui/view/cellsh3.cxx
@@ -816,7 +816,7 @@ void ScCellShell::Execute( SfxRequest& rReq )
                     if ( pReqArgs )
                     {
                         const SfxStringItem& rNameItem = (const SfxStringItem&)pReqArgs->Get( 
SID_AUTOFORMAT );
-                        ScAutoFormat* pFormat = ScGlobal::GetAutoFormat();
+                        ScAutoFormat* pFormat = ScGlobal::GetOrCreateAutoFormat();
                         sal_uInt16 nIndex = pFormat->FindIndexPerName( rNameItem.GetValue() );
 
                         pTabViewShell->AutoFormat( nIndex );
@@ -831,7 +831,7 @@ void ScCellShell::Execute( SfxRequest& rReq )
                         ScAbstractDialogFactory* pFact = ScAbstractDialogFactory::Create();
                         OSL_ENSURE(pFact, "ScAbstractFactory create fail!");
 
-                        AbstractScAutoFormatDlg* pDlg = pFact->CreateScAutoFormatDlg( pDlgParent, 
ScGlobal::GetAutoFormat(), pNewEntry,GetViewData()->GetDocument(), RID_SCDLG_AUTOFORMAT );
+                        AbstractScAutoFormatDlg* pDlg = pFact->CreateScAutoFormatDlg( pDlgParent, 
ScGlobal::GetOrCreateAutoFormat(), pNewEntry,GetViewData()->GetDocument(), RID_SCDLG_AUTOFORMAT );
                         OSL_ENSURE(pDlg, "Dialog create fail!");
 
                         if ( pDlg->Execute() == RET_OK )
-- 
1.7.7.4