Assert on valid order of which ids in ranges on SfxItemSet creation

This allows to make sure we actually use sorted which ranges,
and then it's safe to call SfxItemSet::MergeRange when needed.

Also this change relaxes the previous requirement that ranges
must be separated by at least one; this allows to have adjacent
ranges, like in

RES_FRMATR_BEGIN, RES_FRMATR_END-1,
RES_GRFATR_BEGIN, RES_GRFATR_END-1,

where RES_FRMATR_END is equal to RES_GRFATR_BEGIN. Allowing this
makes possible to (1) self-document the ranges, so it's clear
which ranges are included; and (2) be safe in case when these
constants would change, so that the one merged range would not
unexpectedly contain everything inserted between RES_FRMATR_END
and RES_GRFATR_BEGIN.

Change-Id: Iaad0f099b85059b3aa318a347aa7fbd3f6d455c7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/116909
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/117106
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>

2 files changed, 18 insertions, 27 deletions

diff --git a/sfx2/source/control/bindings.cxx b/sfx2/source/control/bindings.cxx
index d648fdf87537..1792e330f5c4 100644
--- a/sfx2/source/control/bindings.cxx
+++ b/sfx2/source/control/bindings.cxx
@@ -1162,21 +1162,18 @@ std::unique_ptr<SfxItemSet> SfxBindings::CreateSet_Impl
     }
 
     // Create a Set from the ranges
-    std::unique_ptr<sal_uInt16[]> pRanges(new sal_uInt16[rFound.size() * 2 + 1]);
-    int j = 0;
     size_t i = 0;
+    auto pSet(std::make_unique<SfxItemSet>(rPool, nullptr));
     while ( i < rFound.size() )
     {
-        pRanges[j++] = rFound[i].nWhichId;
+        const sal_uInt16 nWhich1 = rFound[i].nWhichId;
             // consecutive numbers
         for ( ; i < rFound.size()-1; ++i )
             if ( rFound[i].nWhichId+1 != rFound[i+1].nWhichId )
                 break;
-        pRanges[j++] = rFound[i++].nWhichId;
+        const sal_uInt16 nWhich2 = rFound[i++].nWhichId;
+        pSet->MergeRange(nWhich1, nWhich2);
     }
-    pRanges[j] = 0; // terminating NULL
-    std::unique_ptr<SfxItemSet> pSet(new SfxItemSet(rPool, pRanges.get()));
-    pRanges.reset();
     return pSet;
 }
 
diff --git a/sfx2/source/dialog/tabdlg.cxx b/sfx2/source/dialog/tabdlg.cxx
index 484b431a690e..c18139dbc347 100644
--- a/sfx2/source/dialog/tabdlg.cxx
+++ b/sfx2/source/dialog/tabdlg.cxx
@@ -687,7 +687,7 @@ const sal_uInt16* SfxTabDialogController::GetInputRanges(const SfxItemPool& rPoo
 
     if ( m_pRanges )
         return m_pRanges.get();
-    std::vector<sal_uInt16> aUS;
+    SfxItemSet aUS(const_cast<SfxItemPool&>(rPool));
 
     for (auto const& elem : m_pImpl->aData)
     {
@@ -695,30 +695,24 @@ const sal_uInt16* SfxTabDialogController::GetInputRanges(const SfxItemPool& rPoo
         if ( elem->fnGetRanges )
         {
             const sal_uInt16* pTmpRanges = (elem->fnGetRanges)();
-            const sal_uInt16* pIter = pTmpRanges;
 
-            sal_uInt16 nLen;
-            for( nLen = 0; *pIter; ++nLen, ++pIter )
-                ;
-            aUS.insert( aUS.end(), pTmpRanges, pTmpRanges + nLen );
+            for (const sal_uInt16* pIter = pTmpRanges; *pIter;)
+            {
+                sal_uInt16 nWidFrom = rPool.GetWhich(*pIter++);
+                assert(*pIter);
+                sal_uInt16 nWidTo = rPool.GetWhich(*pIter++);
+                aUS.MergeRange(nWidFrom, nWidTo); // Keep it valid
+            }
         }
     }
 
-    //! Remove duplicated Ids?
-    {
-        for (auto & elem : aUS)
-            elem = rPool.GetWhich(elem);
-    }
-
-    // sort
-    if ( aUS.size() > 1 )
-    {
-        std::sort( aUS.begin(), aUS.end() );
-    }
+    int nSize = 0;
+    for (const sal_uInt16* pIter = aUS.GetRanges(); pIter && *pIter; pIter++)
+        ++nSize;
 
-    m_pRanges.reset(new sal_uInt16[aUS.size() + 1]);
-    std::copy( aUS.begin(), aUS.end(), m_pRanges.get() );
-    m_pRanges[aUS.size()] = 0;
+    m_pRanges.reset(new sal_uInt16[nSize + 1]);
+    std::copy(aUS.GetRanges(), aUS.GetRanges() + nSize, m_pRanges.get());
+    m_pRanges[nSize] = 0;
     return m_pRanges.get();
 }