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>

3 files changed, 49 insertions, 34 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 )