[Libreoffice-commits] core.git: Branch 'distro/collabora/co-2021' - framework/inc framework/source

[Michael Stahl (via logerrit)]

 framework/inc/uielement/toolbarmanager.hxx    |    2 +-
 framework/source/uielement/toolbarmanager.cxx |    8 ++++----
 framework/source/uielement/toolbarmerger.cxx  |    2 ++
 3 files changed, 7 insertions(+), 5 deletions(-)

New commits:
commit 6c2466299c4054cb5a1d740a88da4f18f142e22b
Author:     Michael Stahl <michael.stahl at allotropia.de>
AuthorDate: Mon Jul 19 17:33:04 2021 +0200
Commit:     Szymon Kłos <szymon.klos at collabora.com>
CommitDate: Thu Jul 22 10:48:21 2021 +0200

    framework: fix multiple-free in VclToolBarManager::Destroy()
    
    Multiple ImplToolItem with mnId 1000 are inserted in the same ToolBox
    and the problem is that only the first one gets its mpUserData set,
    because setting and getting that is based on the assumption that mnId is
    unique per container.
    
    So in Destroy() for every item with mnId 1000 the same AddonsParams
    object is deleted.
    
    The problem is that in ToolBarManager::FillToolbar() the local nItemId
    is copied, so its value never changes when iterating the loop because
    only its copy is incremented.
    
    (regression from ac1aefd5174258d9bcb41465ce566ed1a0819f3d)
    
    Change-Id: Ica47051f9447c2b9d1981f8373d754874f0f31b0
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/119300
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Szymon Kłos <szymon.klos at collabora.com>

diff --git a/framework/inc/uielement/toolbarmanager.hxx b/framework/inc/uielement/toolbarmanager.hxx
index 22e8c728534d..ea66b4cc8525 100644
--- a/framework/inc/uielement/toolbarmanager.hxx
+++ b/framework/inc/uielement/toolbarmanager.hxx
@@ -93,7 +93,7 @@ public:
     virtual vcl::ImageType GetImageSize() = 0;
     virtual void ConnectCallbacks(ToolBarManager* pManager) = 0;
     virtual void SetMenuType(ToolBoxMenuType eType) = 0;
-    virtual void MergeToolbar(sal_uInt16 nItemId,
+    virtual void MergeToolbar(sal_uInt16 & rItemId,
                               const OUString& rModuleIdentifier,
                               CommandToInfoMap& rCommandMap,
                               MergeToolbarInstruction& rInstruction) = 0;
diff --git a/framework/source/uielement/toolbarmanager.cxx b/framework/source/uielement/toolbarmanager.cxx
index cc2452c8edf6..9cdbde0fe926 100644
--- a/framework/source/uielement/toolbarmanager.cxx
+++ b/framework/source/uielement/toolbarmanager.cxx
@@ -333,7 +333,7 @@ public:
         m_pToolBar->SetMenuType( eType );
     }
 
-    virtual void MergeToolbar(sal_uInt16 nItemId,
+    virtual void MergeToolbar(sal_uInt16 & rItemId,
                               const OUString& rModuleIdentifier,
                               CommandToInfoMap& rCommandMap,
                               MergeToolbarInstruction& rInstruction) override
@@ -349,7 +349,7 @@ public:
         {
             ToolBarMerger::ProcessMergeOperation( m_pToolBar,
                                                     aRefPoint.nPos,
-                                                    nItemId,
+                                                    rItemId,
                                                     rCommandMap,
                                                     rModuleIdentifier,
                                                     rInstruction.aMergeCommand,
@@ -359,7 +359,7 @@ public:
         else
         {
             ToolBarMerger::ProcessMergeFallback( m_pToolBar,
-                                                    nItemId,
+                                                    rItemId,
                                                     rCommandMap,
                                                     rModuleIdentifier,
                                                     rInstruction.aMergeCommand,
@@ -518,7 +518,7 @@ public:
 
     virtual void SetMenuType(ToolBoxMenuType /*eType*/) override {}
 
-    virtual void MergeToolbar(sal_uInt16 /*nItemId*/,
+    virtual void MergeToolbar(sal_uInt16 & /*rItemId*/,
                               const OUString& /*rModuleIdentifier*/,
                               CommandToInfoMap& /*rCommandMap*/,
                               MergeToolbarInstruction& /*rInstruction*/) override {}
diff --git a/framework/source/uielement/toolbarmerger.cxx b/framework/source/uielement/toolbarmerger.cxx
index 4a56afe933d9..ce1e3dceb6ef 100644
--- a/framework/source/uielement/toolbarmerger.cxx
+++ b/framework/source/uielement/toolbarmerger.cxx
@@ -619,6 +619,8 @@ void ToolBarMerger::RemoveItems(
 
 void ToolBarMerger::CreateToolbarItem( ToolBox* pToolbar, ToolBox::ImplToolItems::size_type nPos, sal_uInt16 nItemId, const AddonToolbarItem& rItem )
 {
+    assert(pToolbar->GetItemData(nItemId) == nullptr); // that future would contain a double free
+
     pToolbar->InsertItem( nItemId, rItem.aLabel, ToolBoxItemBits::NONE, nPos );
     pToolbar->SetItemCommand( nItemId, rItem.aCommandURL );
     pToolbar->SetQuickHelpText( nItemId, rItem.aLabel );