tdf#142391 - Store method using 0x13 format only when actually needed

Change-Id: I907d234b20be5e3c7bee0d44407f1bf4c4b49f05
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/149175
Tested-by: Jenkins
Reviewed-by: Andreas Heinisch <andreas.heinisch@yahoo.de>

12 files changed, 97 insertions, 69 deletions

diff --git a/basic/source/classes/image.cxx b/basic/source/classes/image.cxx
index e5f9ac3f5df2..eab5fe9e0905 100644
--- a/basic/source/classes/image.cxx
+++ b/basic/source/classes/image.cxx
@@ -124,11 +124,11 @@ bool SbiImage::Load( SvStream& r, sal_uInt32& nVersion )
         nFlags = static_cast<SbiImageFlags>(nTmpFlags);
         eCharSet = nCharSet;
         eCharSet = GetSOLoadTextEncoding( eCharSet );
-        bBadVer  = ( nVersion > B_CURVERSION );
+        bBadVer  = ( nVersion > B_IMG_VERSION_13 );
         nDimBase = static_cast<sal_uInt16>(lDimBase);
     }
 
-    bool bLegacy = ( nVersion < B_EXT_IMG_VERSION );
+    bool bLegacy = ( nVersion < B_IMG_VERSION_12 );
 
     sal_uInt64 nNext;
     while( ( nNext = r.Tell() ) < nLast )
@@ -373,7 +373,7 @@ done:
 
 bool SbiImage::Save( SvStream& r, sal_uInt32 nVer )
 {
-    bool bLegacy = ( nVer < B_EXT_IMG_VERSION );
+    bool bLegacy = ( nVer < B_IMG_VERSION_12 );
 
     // detect if old code exceeds legacy limits
     // if so, then disallow save
@@ -381,7 +381,7 @@ bool SbiImage::Save( SvStream& r, sal_uInt32 nVer )
     {
         SbiImage aEmptyImg;
         aEmptyImg.aName = aName;
-        aEmptyImg.Save( r, B_LEGACYVERSION );
+        aEmptyImg.Save( r, B_IMG_VERSION_11 );
         return true;
     }
     // First of all the header
@@ -391,11 +391,11 @@ bool SbiImage::Save( SvStream& r, sal_uInt32 nVer )
     eCharSet = GetSOStoreTextEncoding( eCharSet );
     if ( bLegacy )
     {
-        r.WriteInt32( B_LEGACYVERSION );
+        r.WriteInt32( B_IMG_VERSION_11 );
     }
     else
     {
-        r.WriteInt32( B_CURVERSION );
+        r.WriteInt32( nVer );
     }
     r .WriteInt32( eCharSet )
       .WriteInt32( nDimBase )
diff --git a/basic/source/classes/sb.cxx b/basic/source/classes/sb.cxx
index 7f02e65d9b55..64ba5cd20c86 100644
--- a/basic/source/classes/sb.cxx
+++ b/basic/source/classes/sb.cxx
@@ -1847,22 +1847,28 @@ bool StarBASIC::LoadData( SvStream& r, sal_uInt16 nVer )
     return true;
 }
 
-bool StarBASIC::StoreData( SvStream& r ) const
+std::pair<bool, sal_uInt32> StarBASIC::StoreData( SvStream& r ) const
 {
-    if( !SbxObject::StoreData( r ) )
+    auto [bSuccess, nVersion] = SbxObject::StoreData(r);
+    if( !bSuccess )
     {
-        return false;
+        return { false, 0 };
     }
     assert(pModules.size() < SAL_MAX_UINT16);
     r.WriteUInt16( static_cast<sal_uInt16>(pModules.size()));
     for( const auto& rpModule: pModules )
     {
-        if( !rpModule->Store( r ) )
+        const auto& [bSuccessModule, nVersionModule] = rpModule->Store(r);
+        if( !bSuccessModule )
         {
-            return false;
+            return { false, 0 };
+        }
+        else if (nVersionModule > nVersion)
+        {
+            nVersion = nVersionModule;
         }
     }
-    return true;
+    return { true, nVersion };
 }
 
 bool StarBASIC::GetUNOConstant( const OUString& rName, css::uno::Any& aOut )
diff --git a/basic/source/classes/sbxmod.cxx b/basic/source/classes/sbxmod.cxx
index aaad6cd9bb41..9a4fd841c98d 100644
--- a/basic/source/classes/sbxmod.cxx
+++ b/basic/source/classes/sbxmod.cxx
@@ -1606,7 +1606,7 @@ bool SbModule::LoadData( SvStream& rStrm, sal_uInt16 nVer )
         return false;
     }
     // If the image is in old format, we fix up the method start offsets
-    if ( nImgVer < B_EXT_IMG_VERSION )
+    if ( nImgVer < B_IMG_VERSION_12 )
     {
         fixUpMethodStart( false, p.get() );
         p->ReleaseLegacyBuffer();
@@ -1631,14 +1631,14 @@ bool SbModule::LoadData( SvStream& rStrm, sal_uInt16 nVer )
     return true;
 }
 
-bool SbModule::StoreData( SvStream& rStrm ) const
+std::pair<bool, sal_uInt32> SbModule::StoreData( SvStream& rStrm ) const
 {
     bool bFixup = ( pImage && !pImage->ExceedsLegacyLimits() );
     if ( bFixup )
         fixUpMethodStart( true );
-    bool bRet = SbxObject::StoreData( rStrm );
-    if ( !bRet )
-        return false;
+    const auto& [bSuccess, nVersion] = SbxObject::StoreData(rStrm);
+    if (!bSuccess)
+        return { false, 0 };
 
     if( pImage )
     {
@@ -1650,10 +1650,10 @@ bool SbModule::StoreData( SvStream& rStrm ) const
         // It should be noted that it probably isn't necessary
         // It would be better not to store the image ( more flexible with
         // formats )
-        bool bRes = pImage->Save( rStrm, B_LEGACYVERSION );
+        bool bRes = pImage->Save( rStrm, nVersion );
         if ( bFixup )
             fixUpMethodStart( false ); // restore method starts
-        return bRes;
+        return { bRes, nVersion };
 
     }
     else
@@ -1663,7 +1663,7 @@ bool SbModule::StoreData( SvStream& rStrm ) const
         aImg.aComment = aComment;
         aImg.aName = GetName();
         rStrm.WriteUChar( 1 );
-        return aImg.Save( rStrm );
+        return { aImg.Save(rStrm, nVersion), nVersion };
     }
 }
 
@@ -1767,7 +1767,8 @@ void SbModule::StoreBinaryData( SvStream& rStrm )
     if (!Compile())
         return;
 
-    if (!SbxObject::StoreData(rStrm))
+    const auto& [bSuccess, nVersion] = SbxObject::StoreData(rStrm);
+    if (!bSuccess)
         return;
 
     pImage->aOUSource.clear();
@@ -1775,7 +1776,7 @@ void SbModule::StoreBinaryData( SvStream& rStrm )
     pImage->aName = GetName();
 
     rStrm.WriteUChar(1);
-    pImage->Save(rStrm);
+    pImage->Save(rStrm, nVersion);
 
     pImage->aOUSource = aOUSource;
 }
@@ -1916,15 +1917,16 @@ bool SbJScriptModule::LoadData( SvStream& rStrm, sal_uInt16 )
     return true;
 }
 
-bool SbJScriptModule::StoreData( SvStream& rStrm ) const
+std::pair<bool, sal_uInt32> SbJScriptModule::StoreData( SvStream& rStrm ) const
 {
-    if( !SbxObject::StoreData( rStrm ) )
-        return false;
+    const auto& [bSuccess, nVersion] = SbxObject::StoreData(rStrm);
+    if( !bSuccess )
+        return { false, 0 };
 
     // Write the source string
     OUString aTmp = aOUSource;
     rStrm.WriteUniOrByteString( aTmp, osl_getThreadTextEncoding() );
-    return true;
+    return { true, nVersion };
 }
 
 
@@ -2006,16 +2008,23 @@ bool SbMethod::LoadData( SvStream& rStrm, sal_uInt16 nVer )
     return true;
 }
 
-bool SbMethod::StoreData( SvStream& rStrm ) const
+std::pair<bool, sal_uInt32> SbMethod::StoreData( SvStream& rStrm ) const
 {
-    if( !SbxMethod::StoreData( rStrm ) )
-        return false;
+    auto [bSuccess, nVersion] = SbxMethod::StoreData(rStrm);
+    if( !bSuccess )
+        return { false, 0 };
 
     //tdf#94617
-    sal_Int16 nMax = std::numeric_limits<sal_Int16>::max();
-    sal_Int16 nStartTemp = nStart % nMax;
-    sal_uInt16 nDebugFlagsTemp = nStart / nMax;
-    nDebugFlagsTemp |= 0x8000;
+    const sal_Int16 nMax = std::numeric_limits<sal_Int16>::max();
+    // tdf#142391 - store method using binary format 0x13 only when actually needed, i.e.,
+    // when method starts at an offset that would overflow 16 bits
+    const sal_Int16 nStartTemp = nStart % nMax;
+    sal_uInt16 nDebugFlagsTemp = static_cast<sal_uInt16>(nDebugFlags);
+    if (nStart >= nMax)
+    {
+        nDebugFlagsTemp = (nStart / nMax) | 0x8000;
+        nVersion = B_IMG_VERSION_13;
+    }
 
     rStrm.WriteUInt16( nDebugFlagsTemp )
          .WriteInt16( nLine1 )
@@ -2023,7 +2032,7 @@ bool SbMethod::StoreData( SvStream& rStrm ) const
          .WriteInt16( nStartTemp )
          .WriteBool( bInvalid );
 
-    return true;
+    return { true, nVersion };
 }
 
 void SbMethod::GetLineRange( sal_uInt16& l1, sal_uInt16& l2 )
diff --git a/basic/source/inc/filefmt.hxx b/basic/source/inc/filefmt.hxx
index 25ba647ad743..b04b7abc6a9b 100644
--- a/basic/source/inc/filefmt.hxx
+++ b/basic/source/inc/filefmt.hxx
@@ -45,9 +45,9 @@
 //                       new integer type suffix 'b')
 //
 
-#define B_LEGACYVERSION 0x00000011
-#define B_EXT_IMG_VERSION 0x00000012
-#define B_CURVERSION 0x00000013
+#define B_IMG_VERSION_11 0x00000011
+#define B_IMG_VERSION_12 0x00000012
+#define B_IMG_VERSION_13 0x00000013
 
 // The file contains either a module- or a library-record.
 // Those records contain further records. Every record's got
diff --git a/basic/source/inc/image.hxx b/basic/source/inc/image.hxx
index f89776af09b9..2e442398a214 100644
--- a/basic/source/inc/image.hxx
+++ b/basic/source/inc/image.hxx
@@ -79,7 +79,7 @@ public:
     bool Load( SvStream&, sal_uInt32& nVer );
                             // nVer is set to version
                             // of image
-    bool Save( SvStream&, sal_uInt32 = B_CURVERSION );
+    bool Save( SvStream&, sal_uInt32 );
     bool IsError() const            { return bError;    }
 
     const sal_uInt8* GetCode() const { return aCode.data(); }
diff --git a/basic/source/inc/sbjsmod.hxx b/basic/source/inc/sbjsmod.hxx
index 128c0e243e95..4f8584ff08e5 100644
--- a/basic/source/inc/sbjsmod.hxx
+++ b/basic/source/inc/sbjsmod.hxx
@@ -28,7 +28,7 @@
 class SbJScriptModule final : public SbModule
 {
     virtual bool LoadData(SvStream&, sal_uInt16) override;
-    virtual bool StoreData(SvStream&) const override;
+    virtual std::pair<bool, sal_uInt32> StoreData(SvStream&) const override;
 
 public:
     SBX_DECL_PERSIST_NODATA(SBXID_JSCRIPTMOD, 1);
diff --git a/basic/source/sbx/sbxarray.cxx b/basic/source/sbx/sbxarray.cxx
index ec95e5e10602..81ad5bf64ada 100644
--- a/basic/source/sbx/sbxarray.cxx
+++ b/basic/source/sbx/sbxarray.cxx
@@ -26,6 +26,7 @@
 
 #include <cstddef>
 #include <optional>
+#include <filefmt.hxx>
 
 struct SbxVarEntry
 {
@@ -357,7 +358,7 @@ bool SbxArray::LoadData( SvStream& rStrm, sal_uInt16 /*nVer*/ )
     return bRes;
 }
 
-bool SbxArray::StoreData( SvStream& rStrm ) const
+std::pair<bool, sal_uInt32> SbxArray::StoreData( SvStream& rStrm ) const
 {
     sal_uInt32 nElem = 0;
     // Which elements are even defined?
@@ -367,17 +368,24 @@ bool SbxArray::StoreData( SvStream& rStrm ) const
             nElem++;
     }
     rStrm.WriteUInt16( nElem );
+
+    sal_uInt32 nVersion = B_IMG_VERSION_12;
     for( size_t n = 0; n < mVarEntries.size(); n++ )
     {
         const SbxVarEntry& rEntry = mVarEntries[n];
         if (rEntry.mpVar.is() && !(rEntry.mpVar->GetFlags() & SbxFlagBits::DontStore))
         {
             rStrm.WriteUInt16( n );
-            if (!rEntry.mpVar->Store(rStrm))
-                return false;
+            const auto& [bSuccess, nVersionModule] = rEntry.mpVar->Store(rStrm);
+            if (!bSuccess)
+                return { false, 0 };
+            else if (nVersionModule > nVersion)
+            {
+                nVersion = nVersionModule;
+            }
         }
     }
-    return true;
+    return { true, nVersion };
 }
 
 // #100883 Method to set method directly to parameter array
@@ -559,7 +567,7 @@ bool SbxDimArray::LoadData( SvStream& rStrm, sal_uInt16 nVer )
     return SbxArray::LoadData( rStrm, nVer );
 }
 
-bool SbxDimArray::StoreData( SvStream& rStrm ) const
+std::pair<bool, sal_uInt32> SbxDimArray::StoreData( SvStream& rStrm ) const
 {
     assert(m_vDimensions.size() <= sal::static_int_cast<size_t>(std::numeric_limits<sal_Int16>::max()));
     rStrm.WriteInt16( m_vDimensions.size() );
diff --git a/basic/source/sbx/sbxbase.cxx b/basic/source/sbx/sbxbase.cxx
index 9d2129e68a1e..3189977a6b8e 100644
--- a/basic/source/sbx/sbxbase.cxx
+++ b/basic/source/sbx/sbxbase.cxx
@@ -31,6 +31,7 @@
 
 #include <rtl/ustring.hxx>
 #include <sal/log.hxx>
+#include <filefmt.hxx>
 
 // AppData-Structure for SBX:
 
@@ -248,7 +249,7 @@ SbxBaseRef SbxBase::Load( SvStream& rStrm )
     return p;
 }
 
-bool SbxBase::Store( SvStream& rStrm )
+std::pair<bool, sal_uInt32> SbxBase::Store( SvStream& rStrm )
 {
     if( ( nFlags & SbxFlagBits::DontStore ) == SbxFlagBits::NONE )
     {
@@ -258,7 +259,7 @@ bool SbxBase::Store( SvStream& rStrm )
              .WriteUInt16( GetVersion() );
         sal_uInt64 const nOldPos = rStrm.Tell();
         rStrm.WriteUInt32( 0 );
-        bool bRes = StoreData( rStrm );
+        auto [bRes, nVersion] = StoreData(rStrm);
         sal_uInt64 const nNewPos = rStrm.Tell();
         rStrm.Seek( nOldPos );
         rStrm.WriteUInt32( nNewPos - nOldPos );
@@ -267,10 +268,10 @@ bool SbxBase::Store( SvStream& rStrm )
             bRes = false;
         if( bRes )
             bRes = true;
-        return bRes;
+        return { bRes, nVersion };
     }
     else
-        return true;
+        return { true, B_IMG_VERSION_12 };
 }
 
 bool SbxBase::LoadCompleted()
diff --git a/basic/source/sbx/sbxcoll.cxx b/basic/source/sbx/sbxcoll.cxx
index 328115d4f6fa..a29e832fa2a8 100644
--- a/basic/source/sbx/sbxcoll.cxx
+++ b/basic/source/sbx/sbxcoll.cxx
@@ -293,16 +293,16 @@ bool SbxStdCollection::LoadData( SvStream& rStrm, sal_uInt16 nVer )
     return bRes;
 }
 
-bool SbxStdCollection::StoreData( SvStream& rStrm ) const
+std::pair<bool, sal_uInt32> SbxStdCollection::StoreData( SvStream& rStrm ) const
 {
-    bool bRes = SbxCollection::StoreData( rStrm );
+    const auto& [bRes, nVersion] = SbxCollection::StoreData(rStrm);
     if( bRes )
     {
         write_uInt16_lenPrefixed_uInt8s_FromOUString(rStrm, aElemClass,
             RTL_TEXTENCODING_ASCII_US);
         rStrm.WriteBool( bAddRemoveOk );
     }
-    return bRes;
+    return { bRes, nVersion };
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/basic/source/sbx/sbxobj.cxx b/basic/source/sbx/sbxobj.cxx
index 4dd054959823..002abfe38301 100644
--- a/basic/source/sbx/sbxobj.cxx
+++ b/basic/source/sbx/sbxobj.cxx
@@ -31,6 +31,7 @@
 #include <svl/SfxBroadcaster.hxx>
 #include "sbxdec.hxx"
 #include "sbxres.hxx"
+#include <filefmt.hxx>
 
 
 static OUString pNameProp;          // Name-Property
@@ -603,11 +604,11 @@ bool SbxObject::LoadData( SvStream& rStrm, sal_uInt16 nVer )
     return true;
 }
 
-bool SbxObject::StoreData( SvStream& rStrm ) const
+std::pair<bool, sal_uInt32> SbxObject::StoreData( SvStream& rStrm ) const
 {
-    if( !SbxVariable::StoreData( rStrm ) )
+    if( !SbxVariable::StoreData(rStrm).first )
     {
-        return false;
+        return { false, 0 };
     }
     OUString aDfltProp;
     if( pDfltProp )
@@ -622,20 +623,21 @@ bool SbxObject::StoreData( SvStream& rStrm ) const
     rStrm.Seek( nPos );
     rStrm.WriteUInt32( nNew - nPos );
     rStrm.Seek( nNew );
-    if( !pMethods->Store( rStrm ) )
+    const auto& [bSuccess, nVersion] = pMethods->Store( rStrm );
+    if( !bSuccess )
     {
-        return false;
+        return { false, 0 };
     }
-    if( !pProps->Store( rStrm ) )
+    if( !pProps->Store( rStrm ).first )
     {
-        return false;
+        return { false, 0 };
     }
-    if( !pObjs->Store( rStrm ) )
+    if( !pObjs->Store( rStrm ).first )
     {
-        return false;
+        return { false, 0 };
     }
     const_cast<SbxObject*>(this)->SetModified( false );
-    return true;
+    return { true, nVersion };
 }
 
 static bool CollectAttrs( const SbxBase* p, OUString& rRes )
diff --git a/basic/source/sbx/sbxvalue.cxx b/basic/source/sbx/sbxvalue.cxx
index dee531ea28c0..d644ca0c3c73 100644
--- a/basic/source/sbx/sbxvalue.cxx
+++ b/basic/source/sbx/sbxvalue.cxx
@@ -32,6 +32,7 @@
 #include "sbxconv.hxx"
 #include <rtlproto.hxx>
 #include <runtime.hxx>
+#include <filefmt.hxx>
 
 
 ///////////////////////////// constructors
@@ -1466,7 +1467,7 @@ bool SbxValue::LoadData( SvStream& r, sal_uInt16 )
     return true;
 }
 
-    bool SbxValue::StoreData( SvStream& r ) const
+    std::pair<bool, sal_uInt32> SbxValue::StoreData( SvStream& r ) const
     {
         sal_uInt16 nType = sal::static_int_cast< sal_uInt16 >(aData.eType);
         r.WriteUInt16( nType );
@@ -1560,9 +1561,9 @@ bool SbxValue::LoadData( SvStream& r, sal_uInt16 )
                 break;
             default:
                 SAL_WARN( "basic.sbx", "Saving a non-supported data type" );
-                return false;
+                return { false, 0 };
         }
-        return true;
+        return { true, B_IMG_VERSION_12 };
     }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/basic/source/sbx/sbxvar.cxx b/basic/source/sbx/sbxvar.cxx
index b73b93c0733d..effcfc495366 100644
--- a/basic/source/sbx/sbxvar.cxx
+++ b/basic/source/sbx/sbxvar.cxx
@@ -35,6 +35,7 @@
 
 #include <com/sun/star/uno/XInterface.hpp>
 #include <utility>
+#include <filefmt.hxx>
 using namespace com::sun::star::uno;
 
 // SbxVariable
@@ -515,7 +516,7 @@ bool SbxVariable::LoadData( SvStream& rStrm, sal_uInt16 nVer )
     return true;
 }
 
-bool SbxVariable::StoreData( SvStream& rStrm ) const
+std::pair<bool, sal_uInt32> SbxVariable::StoreData( SvStream& rStrm ) const
 {
     rStrm.WriteUChar( 0xFF );      // Marker
     bool bValStore;
@@ -532,16 +533,16 @@ bool SbxVariable::StoreData( SvStream& rStrm ) const
         // So that the method will not be executed in any case!
         // CAST, to avoid const!
         pThis->SetFlag( SbxFlagBits::NoBroadcast );
-        bValStore = SbxValue::StoreData( rStrm );
+        bValStore = SbxValue::StoreData( rStrm ).first;
         pThis->ResetFlag( SbxFlagBits::NoBroadcast );
     }
     else
     {
-        bValStore = SbxValue::StoreData( rStrm );
+        bValStore = SbxValue::StoreData( rStrm ).first;
     }
     if( !bValStore )
     {
-        return false;
+        return { false, 0 };
     }
     write_uInt16_lenPrefixed_uInt8s_FromOUString(rStrm, maName,
                                                       RTL_TEXTENCODING_ASCII_US);
@@ -555,7 +556,7 @@ bool SbxVariable::StoreData( SvStream& rStrm ) const
     {
         rStrm.WriteUChar( 0 );
     }
-    return true;
+    return { true, B_IMG_VERSION_12 };
 }
 
 // SbxInfo