From f923370c820e3bcc26e1f8f7266b93e18148f67d Mon Sep 17 00:00:00 2001 From: Pasukhin Dmitry Date: Tue, 4 Nov 2025 10:55:54 +0000 Subject: [PATCH] Foundation Classes - Modernize NCollection_SparseArrayBase memory handling and style (#804) - Replace malloc/calloc/free with Standard::AllocateOptimal/Standard::Free for OCCT-consistent memory management - Modernize constructor declarations and use nullptr instead of 0 for null pointers - Replace custom swap implementation with std::swap and add noexcept specifications --- .../NCollection/NCollection_SparseArray.hxx | 8 +- .../NCollection_SparseArrayBase.cxx | 73 +++++++++++-------- .../NCollection_SparseArrayBase.hxx | 12 +-- 3 files changed, 55 insertions(+), 38 deletions(-) diff --git a/src/FoundationClasses/TKernel/NCollection/NCollection_SparseArray.hxx b/src/FoundationClasses/TKernel/NCollection/NCollection_SparseArray.hxx index d38da03383..c0485cceef 100644 --- a/src/FoundationClasses/TKernel/NCollection/NCollection_SparseArray.hxx +++ b/src/FoundationClasses/TKernel/NCollection/NCollection_SparseArray.hxx @@ -56,6 +56,8 @@ public: //! Explicit assignment operator NCollection_SparseArray& Assign(const NCollection_SparseArray& theOther) { + if (this == &theOther) + return *this; this->assign(theOther); return *this; } @@ -63,7 +65,7 @@ public: //! Exchange the data of two arrays; //! can be used primarily to move contents of theOther into the new array //! in a fast way (without creation of duplicated data) - void Exchange(NCollection_SparseArray& theOther) { this->exchange(theOther); } + void Exchange(NCollection_SparseArray& theOther) noexcept { this->exchange(theOther); } //! Destructor virtual ~NCollection_SparseArray() { Clear(); } @@ -138,7 +140,7 @@ public: { public: //! Empty constructor - for later Init - ConstIterator() {} + ConstIterator() noexcept {} //! Constructor with initialisation ConstIterator(const NCollection_SparseArray& theVector) @@ -166,7 +168,7 @@ public: { public: //! Empty constructor - for later Init - Iterator() {} + Iterator() noexcept {} //! Constructor with initialisation Iterator(NCollection_SparseArray& theVector) diff --git a/src/FoundationClasses/TKernel/NCollection/NCollection_SparseArrayBase.cxx b/src/FoundationClasses/TKernel/NCollection/NCollection_SparseArrayBase.cxx index 370cf7fb29..00f6e2ad49 100644 --- a/src/FoundationClasses/TKernel/NCollection/NCollection_SparseArrayBase.cxx +++ b/src/FoundationClasses/TKernel/NCollection/NCollection_SparseArrayBase.cxx @@ -15,6 +15,9 @@ #include #include +#include + +#include //================================================================================================= @@ -29,12 +32,13 @@ void NCollection_SparseArrayBase::allocData(const Standard_Size iBlock) while (iBlock >= newNbBlocks) newNbBlocks *= 2; - Standard_Address* newData = (Standard_Address*)malloc(newNbBlocks * sizeof(Standard_Address)); + Standard_Address* newData = + (Standard_Address*)Standard::AllocateOptimal(newNbBlocks * sizeof(Standard_Address)); if (myNbBlocks > 0) memcpy(newData, myData, myNbBlocks * sizeof(Standard_Address)); memset(newData + myNbBlocks, 0, (newNbBlocks - myNbBlocks) * sizeof(Standard_Address)); - free(myData); + Standard::Free(myData); myData = newData; myNbBlocks = newNbBlocks; } @@ -44,15 +48,21 @@ void NCollection_SparseArrayBase::allocData(const Standard_Size iBlock) void NCollection_SparseArrayBase::freeBlock(const Standard_Size iBlock) { Standard_Address& anAddr = myData[iBlock]; - Block aBlock = getBlock(anAddr); + if (!anAddr) + return; + + Block aBlock = getBlock(anAddr); + // Destroy all items in the block for (Standard_Size anInd = 0; anInd < myBlockSize; anInd++) + { if (aBlock.IsSet(anInd)) { destroyItem(getItem(aBlock, anInd)); mySize--; } - free(anAddr); - anAddr = 0; + } + Standard::Free(anAddr); + anAddr = nullptr; } //================================================================================================= @@ -61,13 +71,18 @@ void NCollection_SparseArrayBase::Clear() { // free block data for (Standard_Size iBlock = 0; iBlock < myNbBlocks; iBlock++) + { if (myData[iBlock]) + { freeBlock(iBlock); + } + } - // free blocks and reset counters - free(myData); - myData = 0; + // free blocks array and reset counters + Standard::Free(myData); + myData = nullptr; myNbBlocks = 0; + mySize = 0; // consistency check Standard_ProgramError_Raise_if( @@ -107,7 +122,9 @@ void NCollection_SparseArrayBase::assign(const NCollection_SparseArrayBase& theO Standard_Address& anAddr = myData[iBlock]; if (!anAddr) { - anAddr = calloc(Block::Size(myBlockSize, myItemSize), sizeof(char)); + const Standard_Size aBlockSize = Block::Size(myBlockSize, myItemSize); + anAddr = Standard::AllocateOptimal(aBlockSize); + memset(anAddr, 0, aBlockSize); Block aBlock(getBlock(anAddr)); for (Standard_Size anInd = 0; anInd < myBlockSize; anInd++) if (anOtherBlock.IsSet(anInd)) @@ -165,25 +182,17 @@ void NCollection_SparseArrayBase::assign(const NCollection_SparseArrayBase& theO //================================================================================================= -template -static inline void sswap(T& a, T& b) -{ - T c = a; - a = b; - b = c; -} - -void NCollection_SparseArrayBase::exchange(NCollection_SparseArrayBase& theOther) +void NCollection_SparseArrayBase::exchange(NCollection_SparseArrayBase& theOther) noexcept { if (this == &theOther) return; - // swap fields of this and theOther - sswap(myItemSize, theOther.myItemSize); - sswap(myBlockSize, theOther.myBlockSize); - sswap(myNbBlocks, theOther.myNbBlocks); - sswap(mySize, theOther.mySize); - sswap(myData, theOther.myData); + // swap fields of this and theOther using std::swap for better optimization + std::swap(myItemSize, theOther.myItemSize); + std::swap(myBlockSize, theOther.myBlockSize); + std::swap(myNbBlocks, theOther.myNbBlocks); + std::swap(mySize, theOther.mySize); + std::swap(myData, theOther.myData); } //================================================================================================= @@ -200,16 +209,19 @@ Standard_Address NCollection_SparseArrayBase::setValue(const Standard_Size th // allocate block if necessary Standard_Address& anAddr = myData[iBlock]; if (!anAddr) - anAddr = calloc(Block::Size(myBlockSize, myItemSize), sizeof(char)); - - // get a block - Block aBlock(getBlock(anAddr)); + { + const Standard_Size aBlockSize = Block::Size(myBlockSize, myItemSize); + anAddr = Standard::AllocateOptimal(aBlockSize); + memset(anAddr, 0, aBlockSize); + } - // mark item as defined + // get a block and calculate item index + Block aBlock(getBlock(anAddr)); Standard_Size anInd = theIndex % myBlockSize; Standard_Address anItem = getItem(aBlock, anInd); // either create an item by copy constructor if it is new, or assign it + // Optimize: Set() returns non-zero if bit was not set previously if (aBlock.Set(anInd)) { (*aBlock.Count)++; @@ -217,7 +229,10 @@ Standard_Address NCollection_SparseArrayBase::setValue(const Standard_Size th createItem(anItem, theValue); } else + { + // Item already exists, just copy the value copyItem(anItem, theValue); + } return anItem; } diff --git a/src/FoundationClasses/TKernel/NCollection/NCollection_SparseArrayBase.hxx b/src/FoundationClasses/TKernel/NCollection/NCollection_SparseArrayBase.hxx index 1c3f479ec7..75587fea74 100644 --- a/src/FoundationClasses/TKernel/NCollection/NCollection_SparseArrayBase.hxx +++ b/src/FoundationClasses/TKernel/NCollection/NCollection_SparseArrayBase.hxx @@ -164,7 +164,7 @@ public: // Methods for descendant //! Empty constructor - Standard_EXPORT Iterator(const NCollection_SparseArrayBase* theArray = 0); + Standard_EXPORT Iterator(const NCollection_SparseArrayBase* theArray = nullptr); //! Initialize by the specified array Standard_EXPORT void init(const NCollection_SparseArrayBase* theArray); @@ -182,9 +182,9 @@ public: friend class Iterator; private: - // Copy constructor and assignment operator are private thus not accessible - NCollection_SparseArrayBase(const NCollection_SparseArrayBase&); - void operator=(const NCollection_SparseArrayBase&); + // Copy constructor and assignment operator are deleted + NCollection_SparseArrayBase(const NCollection_SparseArrayBase&) = delete; + NCollection_SparseArrayBase& operator=(const NCollection_SparseArrayBase&) = delete; protected: // Object life @@ -195,7 +195,7 @@ protected: myBlockSize(theBlockSize), myNbBlocks(0), mySize(0), - myData(0) + myData(nullptr) { } @@ -238,7 +238,7 @@ protected: //! Exchange contents of theOther and this; //! assumes that this and theOther have exactly the same type of arguments - Standard_EXPORT void exchange(NCollection_SparseArrayBase& theOther); + Standard_EXPORT void exchange(NCollection_SparseArrayBase& theOther) noexcept; protected: // Methods to be provided by descendant -- 2.39.5