[Libreoffice-commits] core.git: Branch 'libreoffice-4-4' - formula/source include/formula sc/inc sc/source

[Caolán McNamara]

 formula/source/ui/dlg/FormulaHelper.cxx  |    4 +---
 include/formula/IFunctionDescription.hxx |    5 +++--
 sc/inc/funcdesc.hxx                      |   10 ++++++----
 sc/source/core/data/funcdesc.cxx         |    7 ++++---
 4 files changed, 14 insertions(+), 12 deletions(-)

New commits:
commit 4e3d54fc9542af87d718b24bcd76a0529133f45f
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Thu Jun 11 16:52:09 2015 +0100

    Resolves: tdf#89643 report builder function wizard segfaults
    
    regression from
    
        commit 3d6521280929ecacc53b7c358d29d0b5d31b3462
        CommitDate: Thu Jul 31 22:14:25 2014 +0200
    
            fix memory leak around function descriptions
    
            Found by Lsan.
    
    There are two implementations of getCategory, one (sc) returns a new one each
    time (hence the leak fix) and the other (reportdesign) returns a pointer to one
    that belongs to the manger (hence the crash).
    
    The code in formula really looks to me to expect that the getCategory return a
    pointer that "someone else" needs to look after, i.e. the reportdesign variant
    is the more correct so revert 3d6521280929ecacc53b7c358d29d0b5d31b3462 and to
    fix the leak make the sc own the ScFunctionCategories and just cache them like
    the reportdesign one does
    
    Change-Id: Ifd986301a54b4d20449e864697655cd973e0c4df
    (cherry picked from commit 7c3abee29c742593206b755b20a718c46f0780fa)
    (cherry picked from commit 73107eb3375f1671f549f0467be2812df9223848)
    Reviewed-on: https://gerrit.libreoffice.org/16232
    Reviewed-by: Eike Rathke <erack at redhat.com>
    Tested-by: Eike Rathke <erack at redhat.com>

diff --git a/formula/source/ui/dlg/FormulaHelper.cxx b/formula/source/ui/dlg/FormulaHelper.cxx
index c644856..ab1baa6 100644
--- a/formula/source/ui/dlg/FormulaHelper.cxx
+++ b/formula/source/ui/dlg/FormulaHelper.cxx
@@ -23,8 +23,6 @@
 #include <unotools/charclass.hxx>
 #include <unotools/syslocale.hxx>
 
-#include <boost/scoped_ptr.hpp>
-
 namespace formula
 {
 
@@ -95,7 +93,7 @@ bool FormulaHelper::GetNextFunc( const OUString&  rFormula,
             const sal_uInt32 nCategoryCount = m_pFunctionManager->getCount();
             for(sal_uInt32 j= 0; j < nCategoryCount && !*ppFDesc; ++j)
             {
-                boost::scoped_ptr<const IFunctionCategory> pCategory(m_pFunctionManager->getCategory(j));
+                const IFunctionCategory* pCategory = m_pFunctionManager->getCategory(j);
                 const sal_uInt32 nCount = pCategory->getCount();
                 for(sal_uInt32 i = 0 ; i < nCount; ++i)
                 {
diff --git a/include/formula/IFunctionDescription.hxx b/include/formula/IFunctionDescription.hxx
index 1b37d51..85f42aa 100644
--- a/include/formula/IFunctionDescription.hxx
+++ b/include/formula/IFunctionDescription.hxx
@@ -60,7 +60,7 @@ namespace formula
         ~IFunctionManager() {}
     };
 
-    class IFunctionCategory
+    class SAL_NO_VTABLE IFunctionCategory
     {
     public:
         IFunctionCategory(){}
@@ -70,7 +70,8 @@ namespace formula
         virtual sal_uInt32                  getNumber() const = 0;
         virtual OUString             getName() const = 0;
 
-        virtual ~IFunctionCategory() {}
+    protected:
+        ~IFunctionCategory() {}
     };
 
     class SAL_NO_VTABLE IFunctionDescription
diff --git a/sc/inc/funcdesc.hxx b/sc/inc/funcdesc.hxx
index e746462..43e4101 100644
--- a/sc/inc/funcdesc.hxx
+++ b/sc/inc/funcdesc.hxx
@@ -27,6 +27,7 @@
 #include <formula/IFunctionDescription.hxx>
 #include <sal/types.h>
 #include <rtl/ustring.hxx>
+#include <map>
 
 #define MAX_FUNCCAT 12  /* maximum number of categories for functions */
 #define LRU_MAX 10 /* maximal number of last recently used functions */
@@ -361,7 +362,7 @@ public:
     /**
       Returns a category.
 
-      Creates an IFunctionCategory object from a category specified by nPos.
+      Returns an IFunctionCategory object for a category specified by nPos.
 
       @param nPos
       the index of the category, note that 0 maps to the first category not the cumulative ('All') category.
@@ -399,9 +400,10 @@ public:
 
 private:
     ScFunctionList* pFuncList; /**< list of all calc functions */
-    ::std::vector<const ScFuncDesc*>* aCatLists[MAX_FUNCCAT]; /**< array of all categories, 0 is the cumulative ('All') category */
-    mutable ::std::vector<const ScFuncDesc*>::iterator pCurCatListIter; /**< position in current category */
-    mutable ::std::vector<const ScFuncDesc*>::iterator pCurCatListEnd; /**< end of current category */
+    std::vector<const ScFuncDesc*>* aCatLists[MAX_FUNCCAT]; /**< array of all categories, 0 is the cumulative ('All') category */
+    mutable std::map< sal_uInt32, std::shared_ptr<ScFunctionCategory> > m_aCategories; /**< map of category pos to IFunctionCategory */
+    mutable std::vector<const ScFuncDesc*>::iterator pCurCatListIter; /**< position in current category */
+    mutable std::vector<const ScFuncDesc*>::iterator pCurCatListEnd; /**< end of current category */
 };
 
 #endif // INCLUDED_SC_INC_FUNCDESC_HXX
diff --git a/sc/source/core/data/funcdesc.cxx b/sc/source/core/data/funcdesc.cxx
index 9dd713d..d9d0266 100644
--- a/sc/source/core/data/funcdesc.cxx
+++ b/sc/source/core/data/funcdesc.cxx
@@ -752,12 +752,13 @@ sal_uInt32 ScFunctionMgr::getCount() const
 
 const formula::IFunctionCategory* ScFunctionMgr::getCategory(sal_uInt32 nCategory) const
 {
-    formula::IFunctionCategory* pRet = NULL;
     if ( nCategory < (MAX_FUNCCAT-1) )
     {
-         pRet = new ScFunctionCategory(const_cast<ScFunctionMgr*>(this),aCatLists[nCategory+1],nCategory); // aCatLists[0] is "all"
+        if (m_aCategories.find(nCategory) == m_aCategories.end())
+            m_aCategories[nCategory].reset(new ScFunctionCategory(const_cast<ScFunctionMgr*>(this),aCatLists[nCategory+1],nCategory)); // aCatLists[0] is "all"
+        return m_aCategories[nCategory].get();
     }
-    return pRet;
+    return NULL;
 }
 
 const formula::IFunctionDescription* ScFunctionMgr::getFunctionByName(const OUString& _sFunctionName) const