Use std::vector in SbiBuffer

This removes memory management from the class; unifies the types
used throughout the code using it; and simplifies it all greatly.

Also this changes errors handling. Prevoiusly setting errors to
parser didn't handle case when no parser was passed to SbiBuffer
ctor, as BufferTransformer did, and could dereference nullptr.

Change-Id: I068eb1b3e9a616a5744fc8233781c4dd4403c84d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/106452
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

3 files changed, 50 insertions, 164 deletions

diff --git a/basic/source/comp/buffer.cxx b/basic/source/comp/buffer.cxx
index b0d87bdaef4d..3d0880f7c678 100644
--- a/basic/source/comp/buffer.cxx
+++ b/basic/source/comp/buffer.cxx
@@ -18,93 +18,54 @@
  */
 
 #include <buffer.hxx>
-#include <parser.hxx>
 
-#include <basic/sberrors.hxx>
-
-const sal_uInt32 UP_LIMIT=0xFFFFFF00;
-
-// The SbiBuffer will be expanded in increments of at least 16 Bytes.
-// This is necessary, because many classes emanate from a buffer length
-// of x*16 Bytes.
-
-SbiBuffer::SbiBuffer( SbiParser* p, short n )
-    : pParser(p)
-    , pCur(nullptr)
-    , nOff(0)
-    , nSize(0)
+namespace
 {
-    n = ( (n + 15 ) / 16 ) * 16;
-    if( !n ) n = 16;
-    nInc  = n;
-}
+const sal_uInt32 UP_LIMIT=0xFFFFFF00;
 
-SbiBuffer::~SbiBuffer()
+template <class I, typename T> void write(I it, T n)
 {
+    *it = static_cast<sal_uInt8>(n & 0xFF);
+    if constexpr (sizeof(n) > 1)
+    {
+        for (std::size_t i = 1; i < sizeof(n); ++i)
+        {
+            n >>= 8;
+            *++it = static_cast<sal_uInt8>(n & 0xFF);
+        }
+    }
 }
-
-// Reach out the buffer
-// This lead to the deletion of the buffer!
-
-char* SbiBuffer::GetBuffer()
-{
-    char* p = pBuf.release();
-    pCur = nullptr;
-    return p;
 }
 
-// Test, if the buffer can contain n Bytes.
-// In case of doubt it will be enlarged
-
-bool SbiBuffer::Check( sal_Int32 n )
+template <typename T> void SbiBuffer::append(T n)
 {
-    if( !n )
-    {
-        return true;
-    }
-    if( nOff + n  >  nSize )
+    if (m_aErrCode)
+        return;
+    if ((m_aBuf.size() + sizeof(n)) > UP_LIMIT)
     {
-        if( nInc == 0 )
-        {
-            return false;
-        }
-
-        sal_Int32 nn = 0;
-        while( nn < n )
-        {
-            nn = nn + nInc;
-        }
-        if( ( nSize + nn ) > UP_LIMIT )
-        {
-            pParser->Error( ERRCODE_BASIC_PROG_TOO_LARGE );
-            nInc = 0;
-            pBuf.reset();
-            return false;
-        }
-        auto p(std::make_unique<char[]>(nSize + nn));
-        if (nSize)
-            memcpy(p.get(), pBuf.get(), nSize);
-        pBuf = std::move(p);
-        pCur = pBuf.get() + nOff;
-        nSize += nn;
+        m_aErrCode = ERRCODE_BASIC_PROG_TOO_LARGE;
+        m_aBuf.clear();
+        return;
     }
-    return true;
+    m_aBuf.reserve(m_aBuf.size() + sizeof(n));
+    write(std::back_inserter(m_aBuf), n);
 }
 
+void SbiBuffer::operator+=(sal_Int8 n) { append(n); }
+void SbiBuffer::operator+=(sal_Int16 n) { append(n); }
+void SbiBuffer::operator+=(sal_uInt8 n) { append(n); }
+void SbiBuffer::operator+=(sal_uInt16 n) { append(n); }
+void SbiBuffer::operator+=(sal_uInt32 n) { append(n); }
+void SbiBuffer::operator+=(sal_Int32 n) { append(n); }
+
 // Patch of a Location
 
 void SbiBuffer::Patch( sal_uInt32 off, sal_uInt32 val )
 {
-    if( ( off + sizeof( sal_uInt32 ) ) < nOff )
-    {
-        sal_uInt16 val1 = static_cast<sal_uInt16>( val & 0xFFFF );
-        sal_uInt16 val2 = static_cast<sal_uInt16>( val >> 16 );
-        sal_uInt8* p = reinterpret_cast<sal_uInt8*>(pBuf.get()) + off;
-        *p++ = static_cast<char>( val1 & 0xFF );
-        *p++ = static_cast<char>( val1 >> 8 );
-        *p++ = static_cast<char>( val2 & 0xFF );
-        *p   = static_cast<char>( val2 >> 8 );
-    }
+    if (m_aErrCode)
+        return;
+    if ((off + sizeof(sal_uInt32)) <= GetSize())
+        write(m_aBuf.begin() + off, val);
 }
 
 // Forward References upon label and procedures
@@ -113,98 +74,20 @@ void SbiBuffer::Patch( sal_uInt32 off, sal_uInt32 val )
 
 void SbiBuffer::Chain( sal_uInt32 off )
 {
-    if( !(off && pBuf) )
+    if (m_aErrCode)
         return;
-
-    sal_uInt8 *ip;
-    sal_uInt32 i = off;
-    sal_uInt32 val1 = (nOff & 0xFFFF);
-    sal_uInt32 val2 = (nOff >> 16);
-    do
+    for (sal_uInt32 i = off; i;)
     {
-        ip = reinterpret_cast<sal_uInt8*>(pBuf.get()) + i;
-        sal_uInt8* pTmp = ip;
-        i =  *pTmp++; i |= *pTmp++ << 8; i |= *pTmp++ << 16; i |= *pTmp++ << 24;
-
-        if( i >= nOff )
+        if ((i + sizeof(sal_uInt32)) > GetSize())
         {
-            pParser->Error( ERRCODE_BASIC_INTERNAL_ERROR, "BACKCHAIN" );
+            m_aErrCode = ERRCODE_BASIC_INTERNAL_ERROR;
+            m_sErrMsg = OUStringLiteral(u"BACKCHAIN");
             break;
         }
-        *ip++ = static_cast<char>( val1 & 0xFF );
-        *ip++ = static_cast<char>( val1 >> 8 );
-        *ip++ = static_cast<char>( val2 & 0xFF );
-        *ip   = static_cast<char>( val2 >> 8 );
-    } while( i );
-}
-
-void SbiBuffer::operator +=( sal_Int8 n )
-{
-    if( Check( 1 ) )
-    {
-        *pCur++ = static_cast<char>(n);
-        nOff += 1;
-    }
-}
-
-bool SbiBuffer::operator +=( sal_uInt8 n )
-{
-    if( Check( 1 ) )
-    {
-        *pCur++ = static_cast<char>(n);
-        nOff += 1;
-        return true;
-    }
-    else
-    {
-        return false;
-    }
-}
-
-void SbiBuffer::operator +=( sal_Int16 n )
-{
-    if( Check( 2 ) )
-    {
-        *pCur++ = static_cast<char>( n & 0xFF );
-        *pCur++ = static_cast<char>( n >> 8 );
-        nOff += 2;
-    }
-}
-
-bool SbiBuffer::operator +=( sal_uInt16 n )
-{
-    if( Check( 2 ) )
-    {
-        *pCur++ = static_cast<char>( n & 0xFF );
-        *pCur++ = static_cast<char>( n >> 8 );
-        nOff += 2;
-        return true;
-    }
-    else
-    {
-        return false;
-    }
-}
-
-bool SbiBuffer::operator +=( sal_uInt32 n )
-{
-    if( Check( 4 ) )
-    {
-        sal_uInt16 n1 = static_cast<sal_uInt16>( n & 0xFFFF );
-        sal_uInt16 n2 = static_cast<sal_uInt16>( n >> 16 );
-        operator +=(n1) && operator +=(n2);
-        return true;
-    }
-    else
-    {
-        return false;
+        auto ip = m_aBuf.begin() + i;
+        i = ip[0] | (ip[1] << 8) | (ip[2] << 16) | (ip[3] << 24);
+        write(ip, GetSize());
     }
 }
 
-void SbiBuffer::operator +=( sal_Int32 n )
-{
-    operator +=( static_cast<sal_uInt32>(n) );
-}
-
-
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/basic/source/comp/codegen.cxx b/basic/source/comp/codegen.cxx
index b20d22c72078..87daa849f74f 100644
--- a/basic/source/comp/codegen.cxx
+++ b/basic/source/comp/codegen.cxx
@@ -36,10 +36,9 @@
 
 // nInc is the increment size of the buffers
 
-SbiCodeGen::SbiCodeGen( SbModule& r, SbiParser* p, short nInc )
+SbiCodeGen::SbiCodeGen(SbModule& r, SbiParser* p)
     : pParser(p)
     , rMod(r)
-    , aCode(p, nInc)
     , nLine(0)
     , nCol(0)
     , nForLevel(0)
@@ -356,8 +355,12 @@ void SbiCodeGen::Save()
             }   // for( iPass...
         }
     }
+    if (aCode.GetErrCode())
+    {
+        pParser->Error(aCode.GetErrCode(), aCode.GetErrMessage());
+    }
     // The code
-    p->AddCode( std::unique_ptr<char[]>(aCode.GetBuffer()), aCode.GetSize() );
+    p->AddCode(aCode.GetBuffer());
 
     // The global StringPool. 0 is not occupied.
     SbiStringPool* pPool = &pParser->aGblStrings;
@@ -498,7 +501,7 @@ class BufferTransformer : public PCodeVisitor< T >
     const sal_uInt8* m_pStart;
     SbiBuffer m_ConvertedBuf;
 public:
-    BufferTransformer():m_pStart(nullptr), m_ConvertedBuf( nullptr, 1024 ) {}
+    BufferTransformer():m_pStart(nullptr) {}
     virtual void start( const sal_uInt8* pStart ) override { m_pStart = pStart; }
     virtual void processOpCode0( SbiOpcode eOp ) override
     {
@@ -576,8 +579,8 @@ PCodeBuffConvertor<T,S>::convert()
     PCodeBufferWalker< T > aBuf( m_pStart, m_nSize );
     BufferTransformer< T, S > aTrnsfrmer;
     aBuf.visitBuffer( aTrnsfrmer );
-    m_pCnvtdBuf = reinterpret_cast<sal_uInt8*>(aTrnsfrmer.buffer().GetBuffer());
-    m_nCnvtdSize = static_cast<S>( aTrnsfrmer.buffer().GetSize() );
+    // TODO: handle buffer errors
+    m_aCnvtdBuf = aTrnsfrmer.buffer().GetBuffer();
 }
 
 template class PCodeBuffConvertor< sal_uInt16, sal_uInt32 >;
diff --git a/basic/source/comp/parser.cxx b/basic/source/comp/parser.cxx
index 9d53c8269113..6ebc2208ae42 100644
--- a/basic/source/comp/parser.cxx
+++ b/basic/source/comp/parser.cxx
@@ -121,7 +121,7 @@ SbiParser::SbiParser( StarBASIC* pb, SbModule* pm )
           aGlobals( aGblStrings, SbGLOBAL, this ),
           aPublics( aGblStrings, SbPUBLIC, this ),
           aRtlSyms( aGblStrings, SbRTL, this ),
-          aGen( *pm, this, 1024 )
+          aGen( *pm, this )
 {
     eEndTok  = NIL;
     pProc    = nullptr;