[Libreoffice-commits] core.git: Branch 'feature/profilesafemode' - comphelper/source configmgr/source desktop/source include/comphelper

Armin Le Grand Armin.Le.Grand at cib.de
Fri Sep 23 16:27:05 UTC 2016


 comphelper/source/misc/backupfilehelper.cxx |  699 ++++++++++++++++++++++------
 configmgr/source/components.cxx             |   18 
 configmgr/source/writemodfile.cxx           |   58 ++
 desktop/source/app/app.cxx                  |   10 
 include/comphelper/backupfilehelper.hxx     |   38 -
 5 files changed, 654 insertions(+), 169 deletions(-)

New commits:
commit 348d6f2aac38101c492d0f92c4778bfaecd46853
Author: Armin Le Grand <Armin.Le.Grand at cib.de>
Date:   Fri Sep 23 18:21:54 2016 +0200

    profilesafe: backups in single *.pack file
    
    Enhanced helper classes for BackupFileHelper to
    allow writing a stack of rescued last valid
    configuration files to a single file package.
    Added configuration values for enabling this and
    defining the number of entries, added max entry
    limitation. Using FileSize and CRC32 now to dectect
    if config file did change. To make this work I added
    sorting to writing the configuration so that with no
    change the same configuration file is written (this
    was not the case before)
    
    Change-Id: I6a175d3d9cb5b947410e8cd00772a521dd81182f

diff --git a/comphelper/source/misc/backupfilehelper.cxx b/comphelper/source/misc/backupfilehelper.cxx
index 7a353b8..628ec2b 100644
--- a/comphelper/source/misc/backupfilehelper.cxx
+++ b/comphelper/source/misc/backupfilehelper.cxx
@@ -9,232 +9,657 @@
 
 #include <sal/config.h>
 
-#include <comphelper/backupfilehelper.hxx>
 #include <rtl/ustring.hxx>
+#include <rtl/bootstrap.hxx>
+#include <comphelper/backupfilehelper.hxx>
+#include <rtl/crc.h>
+#include <deque>
 
-namespace comphelper
-{
-    sal_uInt16 BackupFileHelper::mnMaxAllowedBackups = 10;
-    bool BackupFileHelper::mbExitWasCalled = false;
+typedef std::shared_ptr< osl::File > FileSharedPtr;
 
-    BackupFileHelper::BackupFileHelper(
-        const OUString& rBaseURL,
-        sal_uInt16 nNumBackups)
-        : mrBaseURL(rBaseURL),
-        mnNumBackups(::std::min(::std::max(nNumBackups, sal_uInt16(1)), mnMaxAllowedBackups)),
-        maBase(),
-        maExt(),
-        maBaseFile(rBaseURL),
-        mbBaseFileIsOpen(false)
+namespace
+{
+    OUString splitAtLastToken(const OUString& rSrc, sal_Unicode aToken, OUString& rRight)
     {
-    }
+        const sal_Int32 nIndex(rSrc.lastIndexOf(aToken));
+        OUString aRetval;
 
-    void BackupFileHelper::setExitWasCalled()
-    {
-        mbExitWasCalled = true;
-    }
+        if (-1 == nIndex)
+        {
+            aRetval = rSrc;
+        }
+        else if (nIndex > 0)
+        {
+            aRetval = rSrc.copy(0, nIndex);
+        }
 
-    bool BackupFileHelper::getExitWasCalled()
-    {
-        return mbExitWasCalled;
+        if (rSrc.getLength() > nIndex + 1)
+        {
+            rRight = rSrc.copy(nIndex + 1);
+        }
+
+        return aRetval;
     }
 
-    bool BackupFileHelper::tryPush()
+    sal_uInt32 createCrc32(FileSharedPtr& rCandidate, sal_uInt32 nOffset)
     {
-        if (isDifferentOrNew())
+        sal_uInt32 nCrc32(0);
+
+        if (rCandidate && osl::File::E_None == rCandidate->open(osl_File_OpenFlag_Read))
         {
-            // the new file is different or new, create a new first backup
-            // rename/move/cleanup other backup files, make space for new first backup
-            push();
+            sal_uInt8 aArray[1024];
+            sal_uInt64 nBytesTransfer(0);
+            sal_uInt64 nSize(0);
 
-            // copy new one to now free 1st position
-            osl::File::copy(mrBaseURL, getName(1));
+            rCandidate->getSize(nSize);
 
-            return true;
+            // set offset in source file - should be zero due to crc32 should
+            // only be needed to be created for new entries, gets loaded with old
+            // ones
+            rCandidate->setPos(0, sal_Int64(nOffset));
+
+            while (nSize != 0)
+            {
+                const sal_uInt64 nToTransfer(std::min(nSize, (sal_uInt64)1024));
+
+                if (osl::File::E_None == rCandidate->read(static_cast<void*>(aArray), nToTransfer, nBytesTransfer) && nBytesTransfer == nToTransfer)
+                {
+                    // add to crc and reduce size
+                    nCrc32 = rtl_crc32(nCrc32, static_cast<void*>(aArray), static_cast<sal_uInt32>(nBytesTransfer));
+                    nSize -= nToTransfer;
+                }
+                else
+                {
+                    // error - reset to zero again
+                    nSize = nCrc32 = 0;
+                }
+            }
+
+            rCandidate->close();
         }
 
-        return false;
+        return nCrc32;
     }
+}
 
-    bool BackupFileHelper::isPopPossible()
+namespace
+{
+    struct PackedFileEntry
     {
-        return firstExists();
-    }
+    private:
+        sal_uInt32          mnSize;     // size in bytes
+        sal_uInt32          mnOffset;   // offset in File (zero identifies new file)
+        sal_uInt32          mnCrc32;    // checksum
+        FileSharedPtr       maFile;     // file where to find the data (at offset)
+
+    public:
+        PackedFileEntry(
+            sal_uInt32 nSize,
+            sal_uInt32 nOffset,
+            sal_uInt32 nCrc32,
+            FileSharedPtr& rFile)
+        :   mnSize(nSize),
+            mnOffset(nOffset),
+            mnCrc32(nCrc32),
+            maFile(rFile)
+        {
+        }
 
-    bool BackupFileHelper::tryPop()
-    {
-        if (firstExists())
+        PackedFileEntry()
+        :   mnSize(0),
+            mnOffset(0),
+            mnCrc32(0),
+            maFile()
         {
-            // first copy exists, copy over original and delete
-            const OUString aOneName(getName(1));
-            maBaseFile.close();
-            osl::File::copy(aOneName, mrBaseURL);
-            osl::File::remove(aOneName);
+        }
 
-            // rename/move/cleanup other backup files
-            pop();
+        sal_uInt32 getSize() const
+        {
+            return  mnSize;
+        }
 
-            return true;
+        sal_uInt32 getOffset() const
+        {
+            return mnOffset;
         }
 
-        return false;
-    }
+        sal_uInt32 getCrc32() const
+        {
+            return mnCrc32;
+        }
 
-    rtl::OUString BackupFileHelper::getName(sal_uInt16 n)
-    {
-        if (maExt.isEmpty())
+        bool read_header(
+            FileSharedPtr& rFile,
+            sal_uInt32 nOffset)
         {
-            return OUString(maBase + "_" + OUString::number(n));
+            mnOffset = nOffset;
+            maFile = rFile;
+
+            if (maFile)
+            {
+                sal_uInt8 aArray[4];
+                sal_uInt64 nBaseRead(0);
+
+                // read and compute entry size
+                if (osl::File::E_None == maFile->read(static_cast<void*>(aArray), 4, nBaseRead) && 4 == nBaseRead)
+                {
+                    mnSize = (sal_uInt32(aArray[0]) << 24) + (sal_uInt32(aArray[1]) << 16) + (sal_uInt32(aArray[2]) << 8) + sal_uInt32(aArray[3]);
+                }
+                else
+                {
+                    return false;
+                }
+
+                // read and compute entry crc32
+                if (osl::File::E_None == maFile->read(static_cast<void*>(aArray), 4, nBaseRead) && 4 == nBaseRead)
+                {
+                    mnCrc32 = (sal_uInt32(aArray[0]) << 24) + (sal_uInt32(aArray[1]) << 16) + (sal_uInt32(aArray[2]) << 8) + sal_uInt32(aArray[3]);
+                }
+
+                return true;
+            }
+
+            return false;
         }
 
-        return OUString(maBase + "_" + OUString::number(n) + "." + maExt);
-    }
+        bool write_header(oslFileHandle& rHandle)
+        {
+            sal_uInt8 aArray[4];
+            sal_uInt64 nBaseWritten(0);
 
-    bool BackupFileHelper::firstExists()
-    {
-        if (baseFileOpen() && splitBaseURL())
+            // write size
+            aArray[0] = sal_uInt8((mnSize & 0xff000000) >> 24);
+            aArray[1] = sal_uInt8((mnSize & 0x00ff0000) >> 16);
+            aArray[2] = sal_uInt8((mnSize & 0x0000ff00) >> 8);
+            aArray[3] = sal_uInt8(mnSize & 0x000000ff);
+
+            if (osl::File::E_None != osl_writeFile(rHandle, static_cast<const void*>(aArray), 4, &nBaseWritten) || 4 != nBaseWritten)
+            {
+                return false;
+            }
+
+            // for each entry, write crc32
+            aArray[0] = sal_uInt8((mnCrc32 & 0xff000000) >> 24);
+            aArray[1] = sal_uInt8((mnCrc32 & 0x00ff0000) >> 16);
+            aArray[2] = sal_uInt8((mnCrc32 & 0x0000ff00) >> 8);
+            aArray[3] = sal_uInt8(mnCrc32 & 0x000000ff);
+
+            if (osl::File::E_None != osl_writeFile(rHandle, static_cast<const void*>(aArray), 4, &nBaseWritten) || 4 != nBaseWritten)
+            {
+                return false;
+            }
+
+            return true;
+        }
+
+        bool copy_content(oslFileHandle& rTargetHandle)
         {
-            // check if 1st copy exists
-            osl::File aOneFile(getName(1));
-            const osl::FileBase::RC aResult(aOneFile.open(osl_File_OpenFlag_Read));
+            if (maFile && osl::File::E_None == maFile->open(osl_File_OpenFlag_Read))
+            {
+                sal_uInt8 aArray[1024];
+                sal_uInt64 nBytesTransfer(0);
+                sal_uInt64 nSize(getSize());
+
+                // set offset in source file - when this is zero, a new file is to be added
+                maFile->setPos(0, sal_Int64(getOffset()));
+
+                while (nSize != 0)
+                {
+                    const sal_uInt64 nToTransfer(std::min(nSize, (sal_uInt64)1024));
+
+                    if (osl::File::E_None != maFile->read(static_cast<void*>(aArray), nToTransfer, nBytesTransfer) || nBytesTransfer != nToTransfer)
+                    {
+                        break;
+                    }
 
-            return (osl::File::E_None == aResult);
+                    if (osl::File::E_None != osl_writeFile(rTargetHandle, static_cast<const void*>(aArray), nToTransfer, &nBytesTransfer) || nBytesTransfer != nToTransfer)
+                    {
+                        break;
+                    }
+
+                    nSize -= nToTransfer;
+                }
+
+                maFile->close();
+
+                return (0 == nSize);
+            }
+            else
+            {
+                return false;
+            }
         }
+    };
+}
 
-        return false;
-    }
+namespace
+{
+    typedef ::std::deque< PackedFileEntry > PackedFileEntryVector;
 
-    void BackupFileHelper::pop()
+    class PackedFile
     {
-        for (sal_uInt16 a(2); a < mnMaxAllowedBackups + 1; a++)
+    private:
+        const OUString&         mrURL;
+        PackedFileEntryVector   maPackedFileEntryVector;
+        bool                    mbChanged;
+
+    public:
+        PackedFile(const OUString& rURL)
+        :   mrURL(rURL),
+            maPackedFileEntryVector(),
+            mbChanged(false)
         {
-            const OUString aSourceName(getName(a));
+            FileSharedPtr aSourceFile(new osl::File(rURL));
 
-            if (a > mnNumBackups + 1)
+            if (osl::File::E_None == aSourceFile->open(osl_File_OpenFlag_Read))
             {
-                // try to delete that file, it is out of scope
-                osl::File::remove(aSourceName);
+                sal_uInt64 nBaseLen(0);
+                aSourceFile->getSize(nBaseLen);
+
+                // we need at least File_ID and num entries -> 8byte
+                if (8 < nBaseLen)
+                {
+                    sal_uInt8 aArray[4];
+                    sal_uInt64 nBaseRead(0);
+
+                    // read and check File_ID
+                    if (osl::File::E_None == aSourceFile->read(static_cast< void* >(aArray), 4, nBaseRead) && 4 == nBaseRead)
+                    {
+                        if ('P' == aArray[0] && 'A' == aArray[1] && 'C' == aArray[2] && 'K' == aArray[3])
+                        {
+                            // read and compute num entries in this file
+                            if (osl::File::E_None == aSourceFile->read(static_cast<void*>(aArray), 4, nBaseRead) && 4 == nBaseRead)
+                            {
+                                sal_uInt32 nEntries((sal_uInt32(aArray[0]) << 24) + (sal_uInt32(aArray[1]) << 16) + (sal_uInt32(aArray[2]) << 8) + sal_uInt32(aArray[3]));
+
+                                // if there are entries (and less than max), read them
+                                if (nEntries >= 1 && nEntries <= 10)
+                                {
+                                    // offset in souce file starts with 8Byte for header+numEntries and
+                                    // 8byte for each entry (size and crc32)
+                                    sal_uInt32 nOffset(8 + (8 * nEntries));
+
+                                    for (sal_uInt32 a(0); a < nEntries; a++)
+                                    {
+                                        // create new entry, read header (size and crc) and
+                                        // set offset and source file
+                                        PackedFileEntry aEntry;
+
+                                        if (aEntry.read_header(aSourceFile, nOffset))
+                                        {
+                                            // add to local data
+                                            maPackedFileEntryVector.push_back(aEntry);
+
+                                            // increase offset for next entry
+                                            nOffset += aEntry.getSize();
+                                        }
+                                        else
+                                        {
+                                            // error
+                                            nEntries = 0;
+                                        }
+                                    }
+
+                                    if (0 == nEntries)
+                                    {
+                                        // on read error clear local data
+                                        maPackedFileEntryVector.clear();
+                                    }
+                                }
+                            }
+                        }
+                    }
+                }
+
+                aSourceFile->close();
             }
-            else
+
+            if (maPackedFileEntryVector.empty())
             {
-                // rename that file by decreasing index by one
-                osl::File::move(aSourceName, getName(a - 1));
+                // on error or no data get rid of pack file
+                osl::File::remove(mrURL);
             }
         }
-    }
 
-    void BackupFileHelper::push()
-    {
-        for (sal_uInt16 a(0); a < mnMaxAllowedBackups; a++)
+        bool flush()
+        {
+            bool bRetval(true);
+
+            if (maPackedFileEntryVector.empty())
+            {
+                // get rid of (now?) empty pack file
+                osl::File::remove(mrURL);
+            }
+            else if (mbChanged)
+            {
+                // need to create a new pack file, do this in a temp file to which data
+                // will be copied from local file (so keep it here until this is done)
+                oslFileHandle aHandle;
+                OUString aTempURL;
+
+                // open target temp file
+                if (osl::File::E_None == osl::FileBase::createTempFile(nullptr, &aHandle, &aTempURL))
+                {
+                    sal_uInt8 aArray[4];
+                    sal_uInt64 nBaseWritten(0);
+
+                    aArray[0] = 'P';
+                    aArray[1] = 'A';
+                    aArray[2] = 'C';
+                    aArray[3] = 'K';
+
+                    // write File_ID
+                    if (osl::File::E_None == osl_writeFile(aHandle, static_cast<const void*>(aArray), 4, &nBaseWritten) && 4 == nBaseWritten)
+                    {
+                        const sal_uInt32 nSize(maPackedFileEntryVector.size());
+                        aArray[0] = sal_uInt8((nSize & 0xff000000) >> 24);
+                        aArray[1] = sal_uInt8((nSize & 0x00ff0000) >> 16);
+                        aArray[2] = sal_uInt8((nSize & 0x0000ff00) >> 8);
+                        aArray[3] = sal_uInt8(nSize & 0x000000ff);
+
+                        // write number of entries
+                        if (osl::File::E_None == osl_writeFile(aHandle, static_cast<const void*>(aArray), 4, &nBaseWritten) && 4 == nBaseWritten)
+                        {
+                            // write headers
+                            for (auto& candidateA : maPackedFileEntryVector)
+                            {
+                                if (!candidateA.write_header(aHandle))
+                                {
+                                    // error
+                                    bRetval = false;
+                                    break;
+                                }
+                            }
+
+                            if (bRetval)
+                            {
+                                // write contents
+                                for (auto& candidateB : maPackedFileEntryVector)
+                                {
+                                    if (!candidateB.copy_content(aHandle))
+                                    {
+                                        bRetval = false;
+                                        break;
+                                    }
+                                }
+                            }
+                        }
+                    }
+                }
+
+                // close temp file (in all cases)
+                osl_closeFile(aHandle);
+
+                if (bRetval)
+                {
+                    // copy over existing file by first deleting original
+                    // and moving the temp file to old original
+                    osl::File::remove(mrURL);
+                    osl::File::move(aTempURL, mrURL);
+                }
+
+                // delete temp file (in all cases - it may be moved already)
+                osl::File::remove(aTempURL);
+            }
+
+            return bRetval;
+        }
+
+        bool tryPush(FileSharedPtr& rFileCandidate)
         {
-            const sal_uInt16 nIndex(mnMaxAllowedBackups - a);
-            const OUString aSourceName(getName(nIndex));
+            sal_uInt64 nFileSize(0);
+
+            if (rFileCandidate && osl::File::E_None == rFileCandidate->open(osl_File_OpenFlag_Read))
+            {
+                rFileCandidate->getSize(nFileSize);
+                rFileCandidate->close();
+            }
 
-            if (nIndex >= mnNumBackups)
+            if (0 == nFileSize)
             {
-                // try to delete that file, it is out of scope
-                osl::File::remove(aSourceName);
+                // empty file offered
+                return false;
+            }
+
+            bool bNeedToAdd(false);
+            sal_uInt32 nCrc32(0);
+
+            if (!maPackedFileEntryVector.empty())
+            {
+                // already backups there, check if different from last entry
+                const PackedFileEntry& aLastEntry = maPackedFileEntryVector.back();
+
+                if (aLastEntry.getSize() != static_cast<sal_uInt32>(nFileSize))
+                {
+                    // different size, different file
+                    bNeedToAdd = true;
+                }
+                else
+                {
+                    // same size, check crc32
+                    nCrc32 = createCrc32(rFileCandidate, 0);
+
+                    if (nCrc32 != aLastEntry.getCrc32())
+                    {
+                        // different crc, different file
+                        bNeedToAdd = true;
+                    }
+                }
             }
             else
             {
-                // rename that file by increasing index by one
-                osl::File::move(aSourceName, getName(nIndex + 1));
+                // no backup yet, add
+                bNeedToAdd = true;
+            }
+
+            if (bNeedToAdd)
+            {
+                // create crc32 if not yet done
+                if (0 == nCrc32)
+                {
+                    nCrc32 = createCrc32(rFileCandidate, 0);
+                }
+
+                // create a file entry for a new file. Offset is set to 0 to mark
+                // the entry as new file entry
+                maPackedFileEntryVector.push_back(
+                    PackedFileEntry(
+                        static_cast< sal_uInt32 >(nFileSize),
+                        0,
+                        nCrc32,
+                        rFileCandidate));
+
+                mbChanged = true;
             }
+
+            return bNeedToAdd;
         }
-    }
 
-    bool BackupFileHelper::isDifferentOrNew()
-    {
-        if (baseFileOpen() && splitBaseURL())
+        bool tryPop(oslFileHandle& rHandle)
         {
-            osl::File aLastFile(getName(1));
-            const osl::FileBase::RC aResult(aLastFile.open(osl_File_OpenFlag_Read));
-            bool bDifferentOrNew(false);
+            bool bRetval(false);
 
-            if (osl::File::E_None == aResult)
+            if (!maPackedFileEntryVector.empty())
             {
-                // exists, check for being equal
-                bDifferentOrNew = !equalsBase(aLastFile);
+                // already backups there, check if different from last entry
+                PackedFileEntry& aLastEntry = maPackedFileEntryVector.back();
+
+                bRetval = aLastEntry.copy_content(rHandle);
+
+                if (bRetval)
+                {
+                    maPackedFileEntryVector.pop_back();
+                    mbChanged = true;
+                }
+
+                return bRetval;
             }
-            else if (osl::File::E_NOENT == aResult)
+
+            return false;
+        }
+
+        void tryReduceToNumBackups(sal_uInt16 nNumBackups)
+        {
+            while (maPackedFileEntryVector.size() > nNumBackups)
             {
-                // does not exist - also copy
-                bDifferentOrNew = true;
+                maPackedFileEntryVector.pop_front();
+                mbChanged = true;
             }
+        }
 
-            return bDifferentOrNew;
+        bool empty()
+        {
+            return maPackedFileEntryVector.empty();
         }
+    };
+}
 
-        return false;
+namespace comphelper
+{
+    sal_uInt16 BackupFileHelper::mnMaxAllowedBackups = 10;
+    bool BackupFileHelper::mbExitWasCalled = false;
+
+    BackupFileHelper::BackupFileHelper(
+        const OUString& rBaseURL,
+        sal_uInt16 nNumBackups)
+    :   mrBaseURL(rBaseURL),
+        mnNumBackups(::std::min(::std::max(nNumBackups, sal_uInt16(1)), mnMaxAllowedBackups)),
+        maBase(),
+        maName(),
+        maExt()
+    {
     }
 
-    bool BackupFileHelper::equalsBase(osl::File& rLastFile)
+    void BackupFileHelper::setExitWasCalled()
     {
-        sal_uInt64 nBaseLen(0);
-        sal_uInt64 nLastLen(0);
-        maBaseFile.getSize(nBaseLen);
-        rLastFile.getSize(nLastLen);
+        mbExitWasCalled = true;
+    }
 
-        if (nBaseLen == nLastLen)
+    bool BackupFileHelper::getExitWasCalled()
+    {
+        return mbExitWasCalled;
+    }
+
+    bool BackupFileHelper::getSecureUserConfig(sal_uInt16& rnSecureUserConfigNumCopies)
+    {
+        // init to not active
+        bool bRetval(false);
+        rnSecureUserConfigNumCopies = 0;
+        OUString sTokenOut;
+
+        if (rtl::Bootstrap::get("SecureUserConfig", sTokenOut))
+        {
+            bRetval = sTokenOut.toBoolean();
+        }
+
+        if (bRetval && rtl::Bootstrap::get("SecureUserConfigNumCopies", sTokenOut))
         {
-            // same filesize -> need to check content
-            sal_uInt8 aArrayOld[1024];
-            sal_uInt8 aArrayLast[1024];
-            sal_uInt64 nBytesReadBase(0);
-            sal_uInt64 nBytesReadLast(0);
-            bool bDiffers(false);
+            rnSecureUserConfigNumCopies = static_cast< sal_uInt16 >(sTokenOut.toUInt32());
+        }
 
-            // both rewind on start
-            maBaseFile.setPos(0, 0);
-            rLastFile.setPos(0, 0);
+        return bRetval;
+    }
 
-            while (!bDiffers
-                && osl::File::E_None == maBaseFile.read(static_cast<void*>(aArrayOld), 1024, nBytesReadBase)
-                && osl::File::E_None == rLastFile.read(static_cast<void*>(aArrayLast), 1024, nBytesReadLast)
-                && 0 != nBytesReadBase
-                && nBytesReadBase == nBytesReadLast)
+    rtl::OUString BackupFileHelper::getName()
+    {
+        return OUString(maBase + "/." + maName + ".pack");
+    }
+
+    bool BackupFileHelper::tryPush()
+    {
+        if (splitBaseURL() && baseFileExists())
+        {
+            PackedFile aPackedFile(getName());
+            FileSharedPtr aBaseFile(new osl::File(mrBaseURL));
+
+            if (aPackedFile.tryPush(aBaseFile))
             {
-                bDiffers = memcmp(aArrayOld, aArrayLast, nBytesReadBase);
+                // reduce to allowed number and flush
+                aPackedFile.tryReduceToNumBackups(mnNumBackups);
+                aPackedFile.flush();
+
+                return true;
             }
+        }
 
-            return !bDiffers;
+        return false;
+    }
+
+    bool BackupFileHelper::isPopPossible()
+    {
+        if (splitBaseURL() && baseFileExists())
+        {
+            PackedFile aPackedFile(getName());
+
+            return !aPackedFile.empty();
         }
 
         return false;
     }
 
-    bool BackupFileHelper::splitBaseURL()
+    bool BackupFileHelper::tryPop()
     {
-        if (maBase.isEmpty() && !mrBaseURL.isEmpty())
+        if (splitBaseURL() && baseFileExists())
         {
-            const sal_Int32 nIndex(mrBaseURL.lastIndexOf('.'));
+            PackedFile aPackedFile(getName());
 
-            if (-1 == nIndex)
+            if (!aPackedFile.empty())
             {
-                maBase = mrBaseURL;
-            }
-            else if (nIndex > 0)
-            {
-                maBase = mrBaseURL.copy(0, nIndex);
+                oslFileHandle aHandle;
+                OUString aTempURL;
+
+                // open target temp file
+                if (osl::File::E_None == osl::FileBase::createTempFile(nullptr, &aHandle, &aTempURL))
+                {
+                    bool bRetval(aPackedFile.tryPop(aHandle));
+
+                    // close temp file (in all cases)
+                    osl_closeFile(aHandle);
+
+                    if (bRetval)
+                    {
+                        // copy over existing file by first deleting original
+                        // and moving the temp file to old original
+                        osl::File::remove(mrBaseURL);
+                        osl::File::move(aTempURL, mrBaseURL);
+
+                        // reduce to allowed number and flush
+                        aPackedFile.tryReduceToNumBackups(mnNumBackups);
+                        aPackedFile.flush();
+                    }
+
+                    // delete temp file (in all cases - it may be moved already)
+                    osl::File::remove(aTempURL);
+
+                    return bRetval;
+                }
             }
+        }
 
-            if (mrBaseURL.getLength() > nIndex + 1)
-            {
-                maExt = mrBaseURL.copy(nIndex + 1);
-            }
+        return false;
+    }
+
+    bool BackupFileHelper::splitBaseURL()
+    {
+        if (maBase.isEmpty() && !mrBaseURL.isEmpty())
+        {
+            // split URL at extension and at last path separator
+            maBase = splitAtLastToken(splitAtLastToken(mrBaseURL, '.', maExt), '/', maName);
         }
 
-        return !maBase.isEmpty();
+        return !maBase.isEmpty() && !maName.isEmpty();
     }
 
-    bool BackupFileHelper::baseFileOpen()
+    bool BackupFileHelper::baseFileExists()
     {
-        if (!mbBaseFileIsOpen && !mrBaseURL.isEmpty())
+        if (!mrBaseURL.isEmpty())
         {
-            mbBaseFileIsOpen = (osl::File::E_None == maBaseFile.open(osl_File_OpenFlag_Read));
+            FileSharedPtr aBaseFile(new osl::File(mrBaseURL));
+
+            return (osl::File::E_None == aBaseFile->open(osl_File_OpenFlag_Read));
         }
 
-        return mbBaseFileIsOpen;
+        return false;
     }
 }
 
diff --git a/configmgr/source/components.cxx b/configmgr/source/components.cxx
index 7790ab0..54d8b3c 100644
--- a/configmgr/source/components.cxx
+++ b/configmgr/source/components.cxx
@@ -644,18 +644,22 @@ Components::~Components()
         (*i)->setAlive(false);
     }
 
-    // test backup of registrymodifications
-    static bool bFeatureSecureUserConfig(true);
-
     if (!bExitWasCalled &&
-        bFeatureSecureUserConfig &&
         ModificationTarget::File == modificationTarget_ &&
         !modificationFileUrl_.isEmpty())
     {
-        static sal_uInt16 nNumCopies(5);
-        comphelper::BackupFileHelper aBackupFileHelper(modificationFileUrl_, nNumCopies);
+        // test backup of registrymodifications
+        sal_uInt16 nSecureUserConfigNumCopies(0);
+
+        // read configuration from soffice.ini
+        const bool bSecureUserConfig(comphelper::BackupFileHelper::getSecureUserConfig(nSecureUserConfigNumCopies));
 
-        aBackupFileHelper.tryPush();
+        if (bSecureUserConfig)
+        {
+            comphelper::BackupFileHelper aBackupFileHelper(modificationFileUrl_, nSecureUserConfigNumCopies);
+
+            aBackupFileHelper.tryPush();
+        }
     }
 }
 
diff --git a/configmgr/source/writemodfile.cxx b/configmgr/source/writemodfile.cxx
index 1d20c18..15c9ffa 100644
--- a/configmgr/source/writemodfile.cxx
+++ b/configmgr/source/writemodfile.cxx
@@ -400,6 +400,16 @@ void writeNode(
     }
 }
 
+// helpers to allow sorting of configmgr::Modifications::Node
+typedef std::pair< const rtl::OUString, configmgr::Modifications::Node > aPairEntry;
+struct PairEntrySorter
+{
+    bool operator() (const aPairEntry* pValue1, const aPairEntry* pValue2) const
+    {
+        return pValue1->first.compareTo(pValue2->first) < 0;
+    }
+};
+
 void writeModifications(
     Components & components, TempFile &handle,
     OUString const & parentPathRepresentation,
@@ -459,11 +469,26 @@ void writeModifications(
         OUString pathRep(
             parentPathRepresentation + "/" +
             Data::createSegment(node->getTemplateName(), nodeName));
-        for (const auto & i : modifications.children)
+
+        // copy configmgr::Modifications::Node's to a sortable list. Use pointers
+        // to just reference the data instead of copying it
+        std::list< const aPairEntry* > aPairEntryList;
+
+        for (const auto& rCand : modifications.children)
+        {
+            aPairEntryList.push_back(&rCand);
+        }
+
+        // sort the list
+        aPairEntryList.sort(PairEntrySorter());
+
+        // now use the list to write entries in sorted order
+        // instead of random as from the unordered map
+        for (const auto & i : aPairEntryList)
         {
             writeModifications(
-                components, handle, pathRep, node, i.first,
-                node->getMember(i.first), i.second);
+                components, handle, pathRep, node, i->first,
+                node->getMember(i->first), i->second);
         }
     }
 }
@@ -601,9 +626,30 @@ void writeModFile(
     //TODO: Do not write back information about those removed items that did not
     // come from the .xcs/.xcu files, anyway (but had been added dynamically
     // instead):
-    for (Modifications::Node::Children::const_iterator j(
-             data.modifications.getRoot().children.begin());
-         j != data.modifications.getRoot().children.end(); ++j)
+
+    // For profilesafemode it is necessary to detect changes in the
+    // registrymodifications file, this is done based on file size in bytes and crc32.
+    // Unfortunately this write is based on writing unordered map entries, which creates
+    // valid and semantically equal XML-Files, bubt with different crc32 checksums. For
+    // the future usage it will be preferrable to have easily comparable config files
+    // which is guaranteed by writing the entries in sorted order. Indeed with this change
+    // (and in the recursive writeModifications call) the same config files get written
+
+    // copy configmgr::Modifications::Node's to a sortable list. Use pointers
+    // to just reference the data instead of copying it
+    std::list< const aPairEntry* > aPairEntryList;
+
+    for (const auto& rCand : data.modifications.getRoot().children)
+    {
+        aPairEntryList.push_back(&rCand);
+    }
+
+    // sort the list
+    aPairEntryList.sort(PairEntrySorter());
+
+    // now use the list to write entries in sorted order
+    // instead of random as from the unordered map
+    for (const auto& j : aPairEntryList)
     {
         writeModifications(
             components, tmp, "", rtl::Reference< Node >(), j->first,
diff --git a/desktop/source/app/app.cxx b/desktop/source/app/app.cxx
index b3514ef..16274bc 100644
--- a/desktop/source/app/app.cxx
+++ b/desktop/source/app/app.cxx
@@ -948,11 +948,13 @@ void Desktop::HandleBootstrapErrors(
     else if ( aBootstrapError == BE_OFFICECONFIG_BROKEN )
     {
         // test restore of registrymodifications
-        static bool bFeatureSecureUserConfig(true);
-        static sal_uInt16 nNumCopies(5);
+        sal_uInt16 nSecureUserConfigNumCopies(0);
         bool bFireOriginalError(true);
 
-        if (bFeatureSecureUserConfig)
+        // read configuration from soffice.ini
+        const bool bSecureUserConfig(comphelper::BackupFileHelper::getSecureUserConfig(nSecureUserConfigNumCopies));
+
+        if (bSecureUserConfig)
         {
             // try to asccess user layer configuration file
             OUString conf("${CONFIGURATION_LAYERS}");
@@ -977,7 +979,7 @@ void Desktop::HandleBootstrapErrors(
 
             if (!aUser.isEmpty())
             {
-                comphelper::BackupFileHelper aBackupFileHelper(aUser, nNumCopies);
+                comphelper::BackupFileHelper aBackupFileHelper(aUser, nSecureUserConfigNumCopies);
 
                 if (aBackupFileHelper.isPopPossible())
                 {
diff --git a/include/comphelper/backupfilehelper.hxx b/include/comphelper/backupfilehelper.hxx
index f407d2b..2c6cc25 100644
--- a/include/comphelper/backupfilehelper.hxx
+++ b/include/comphelper/backupfilehelper.hxx
@@ -15,6 +15,7 @@
 #include <comphelper/comphelperdllapi.h>
 #include <rtl/ustring.hxx>
 #include <osl/file.hxx>
+#include <memory>
 
 namespace comphelper
 {
@@ -45,12 +46,11 @@ namespace comphelper
     {
     private:
         // internal data
-        const OUString&     mrBaseURL;
-        sal_uInt16          mnNumBackups;
-        OUString            maBase;
-        OUString            maExt;
-        osl::File           maBaseFile;
-        bool                mbBaseFileIsOpen;
+        const OUString&                 mrBaseURL;
+        sal_uInt16                      mnNumBackups;
+        OUString                        maBase;
+        OUString                        maName;
+        OUString                        maExt;
 
         // internal flag if _exit() was called already - a hint to evtl.
         // not create copies of potentially not well-defined data. This
@@ -67,6 +67,10 @@ namespace comphelper
     public:
         /** Constructor to handle Backups of the given file
          *
+         *  @param  rxContext
+         *          ComponentContext to use internally; needs to be handed
+         *          over due to usages after DeInit() and thus no access
+         *          anymore using comphelper::getProcessComponentContext()
          *  @param  rBaseURL
          *          URL to an existing file that needs to be backed up
          *
@@ -77,12 +81,21 @@ namespace comphelper
          *          It is used in tryPush() and tryPop() calls to cleanup/
          *          reduce the number of existing backups
          */
-        BackupFileHelper(const OUString& rBaseURL, sal_uInt16 nNumBackups = 5);
+        BackupFileHelper(
+            const OUString& rBaseURL,
+            sal_uInt16 nNumBackups = 5);
 
-        // allow to set flag when app had to call _exit()
+        // allow to set static global flag when app had to call _exit()
         static void setExitWasCalled();
         static bool getExitWasCalled();
 
+        // static helper to read config values - these are derived from
+        // soffice.ini due to cui not being available in all cases. The
+        // boolean SecureUserConfig is returned.
+        // Default for SecureUserConfig is false
+        // Default for SecureUserConfigNumCopies is 0 (zero)
+        static bool getSecureUserConfig(sal_uInt16& rnSecureUserConfigNumCopies);
+
         /** tries to create a new backup, if there is none yet, or if the
          *  last differs from the base file. It will then put a new verion
          *  on the 'stack' of copies and evtl. delete the oldest backup.
@@ -113,14 +126,9 @@ namespace comphelper
 
     private:
         // internal helper methods
-        rtl::OUString getName(sal_uInt16 n);
-        bool firstExists();
-        void pop();
-        void push();
-        bool isDifferentOrNew();
-        bool equalsBase(osl::File& rLastFile);
         bool splitBaseURL();
-        bool baseFileOpen();
+        bool baseFileExists();
+        rtl::OUString getName();
     };
 }
 


More information about the Libreoffice-commits mailing list