Merge branch 'fix_niftest_afl_findings' into 'master'

Fix AFL findings in niftest

See merge request OpenMW/openmw!4988

components/bgsm/stream.cpp

@@ -1,5 +1,13 @@
 #include "stream.hpp"
 
+#include <osg/Vec2f>
+#include <osg/Vec3f>
+#include <osg/Vec4f>
+
+#include <format>
+#include <stdexcept>
+#include <string>
+
 namespace Bgsm
 {
     template <>
@@ -30,8 +38,9 @@ namespace Bgsm
             throw std::runtime_error("Requested string length is too large: " + std::to_string(length));
         str = std::string(length, '\0');
         mStream->read(str.data(), length);
-        if (mStream->bad())
+        if (mStream->fail())
-            throw std::runtime_error("Failed to read sized string of " + std::to_string(length) + " chars");
+            throw std::runtime_error(std::format(
+                "Failed to read sized string of {} chars: {}", length, std::generic_category().message(errno)));
         std::size_t end = str.find('\0');
         if (end != std::string::npos)
             str.erase(end);

components/bgsm/stream.hpp

@@ -3,7 +3,7 @@
 
 #include <array>
 #include <cassert>
-#include <cstdint>
+#include <format>
 #include <istream>
 #include <stdexcept>
 #include <string>
@@ -23,9 +23,9 @@ namespace Bgsm
     {
         static_assert(std::is_arithmetic_v<T>, "Buffer element type is not arithmetic");
         pIStream->read(reinterpret_cast<char*>(dest), numInstances * sizeof(T));
-        if (pIStream->bad())
+        if (pIStream->fail())
-            throw std::runtime_error("Failed to read typed (" + std::string(typeid(T).name()) + ") buffer of "
+            throw std::runtime_error(std::format("Failed to read typed ({}) buffer of {} instances: {}",
-                + std::to_string(numInstances) + " instances");
+                typeid(T).name(), numInstances, std::generic_category().message(errno)));
         if constexpr (Misc::IS_BIG_ENDIAN)
             for (std::size_t i = 0; i < numInstances; i++)
                 Misc::swapEndiannessInplace(dest[i]);

components/nif/controller.cpp

@@ -8,6 +8,13 @@
 
 namespace Nif
 {
+    namespace
+    {
+        void readSkinnedShapeGroup(NIFStream& stream, std::vector<NiBoneLODController::SkinInfo>& value)
+        {
+            stream.readVectorOfRecords<uint32_t>(value);
+        }
+    }
 
     void NiTimeController::read(NIFStream* nif)
     {
@@ -84,11 +91,10 @@ namespace Nif
             nif->read(mAccumRootName);
             mTextKeys.read(nif);
         }
-        mControlledBlocks.resize(nif->get<uint32_t>());
+        const uint32_t size = nif->get<uint32_t>();
         if (nif->getVersion() >= NIFStream::generateVersion(10, 1, 0, 106))
             nif->read(mArrayGrowBy);
-        for (ControlledBlock& block : mControlledBlocks)
+        nif->readVectorOfRecords(size, mControlledBlocks);
-            block.read(nif);
     }
 
     void NiSequence::post(Reader& nif)
@@ -122,13 +128,8 @@ namespace Nif
             mStringPalette.read(nif);
         else if (nif->getVersion() >= NIFFile::NIFVersion::VER_BGS && nif->getBethVersion() >= 24)
         {
-            if (nif->getBethVersion() >= 29)
+            const uint16_t size = nif->getBethVersion() >= 29 ? nif->get<uint16_t>() : 1;
-                mAnimNotesList.resize(nif->get<uint16_t>());
+            nif->readVectorOfRecords(size, mAnimNotesList);
-            else
-                mAnimNotesList.resize(1);
-
-            for (auto& notes : mAnimNotesList)
-                notes.read(nif);
         }
     }
 
@@ -482,29 +483,27 @@ namespace Nif
         mData.post(nif);
     }
 
+    void NiBoneLODController::SkinInfo::read(NIFStream* nif)
+    {
+        mShape.read(nif);
+        mSkin.read(nif);
+    }
+
     void NiBoneLODController::read(NIFStream* nif)
     {
         NiTimeController::read(nif);
 
         nif->read(mLOD);
-        mNodeGroups.resize(nif->get<uint32_t>());
+        const uint32_t nodeGroupsCount = nif->get<uint32_t>();
+        mNodeGroups.reserve(nodeGroupsCount);
         nif->read(mNumNodeGroups);
-        for (NiAVObjectList& group : mNodeGroups)
+        for (uint32_t i = 0; i < nodeGroupsCount; ++i)
-            readRecordList(nif, group);
+            readRecordList(nif, mNodeGroups.emplace_back());
 
         if (nif->getBethVersion() != 0 || nif->getVersion() < NIFStream::generateVersion(4, 2, 2, 0))
             return;
 
-        mSkinnedShapeGroups.resize(nif->get<uint32_t>());
+        nif->readVectorOfRecords<uint32_t>(readSkinnedShapeGroup, mSkinnedShapeGroups);
-        for (std::vector<SkinInfo>& group : mSkinnedShapeGroups)
-        {
-            group.resize(nif->get<uint32_t>());
-            for (SkinInfo& info : group)
-            {
-                info.mShape.read(nif);
-                info.mSkin.read(nif);
-            }
-        }
         readRecordList(nif, mShapeGroups);
     }
 
@@ -892,9 +891,7 @@ namespace Nif
 
     void BSTreadTransfInterpolator::read(NIFStream* nif)
     {
-        mTransforms.resize(nif->get<uint32_t>());
+        nif->readVectorOfRecords<uint32_t>(mTransforms);
-        for (BSTreadTransform& transform : mTransforms)
-            transform.read(nif);
         mData.read(nif);
     }
 

components/nif/controller.hpp

@@ -322,6 +322,8 @@ namespace Nif
         {
             NiTriBasedGeomPtr mShape;
             NiSkinInstancePtr mSkin;
+
+            void read(NIFStream* nif);
         };
 
         uint32_t mLOD;

components/nif/data.cpp

@@ -8,6 +8,46 @@
 
 namespace Nif
 {
+    namespace
+    {
+        void readNiSkinDataVertWeight(NIFStream& stream, NiSkinData::VertWeight& value)
+        {
+            auto& [vertex, weight] = value;
+            stream.read(vertex);
+            stream.read(weight);
+        }
+
+        struct ReadNiSkinDataBoneInfo
+        {
+            bool mHasVertexWeights;
+
+            void operator()(NIFStream& stream, NiSkinData::BoneInfo& value) const
+            {
+                stream.read(value.mTransform);
+                stream.read(value.mBoundSphere);
+
+                const uint16_t numVertices = stream.get<uint16_t>();
+
+                if (!mHasVertexWeights)
+                    return;
+
+                stream.readVectorOfRecords(numVertices, readNiSkinDataVertWeight, value.mWeights);
+            }
+        };
+
+        struct ReadNiMorphDataMorphData
+        {
+            uint32_t mNumVerts;
+
+            void operator()(NIFStream& stream, NiMorphData::MorphData& value) const
+            {
+                value.mKeyFrames = std::make_shared<FloatKeyMap>();
+                value.mKeyFrames->read(&stream, /*morph*/ true);
+                stream.readVector(value.mVertices, mNumVerts);
+            }
+        };
+    }
+
     void NiGeometryData::read(NIFStream* nif)
     {
         if (nif->getVersion() >= NIFStream::generateVersion(10, 1, 0, 114))
@@ -215,20 +255,21 @@ namespace Nif
         nif->read(mSigned);
     }
 
+    void NiPixelData::Mipmap::read(NIFStream* nif)
+    {
+        nif->read(mWidth);
+        nif->read(mHeight);
+        nif->read(mOffset);
+    }
+
     void NiPixelData::read(NIFStream* nif)
     {
         mPixelFormat.read(nif);
         mPalette.read(nif);
-        mMipmaps.resize(nif->get<uint32_t>());
+        const uint32_t mipmapsCount = nif->get<uint32_t>();
         nif->read(mBytesPerPixel);
-        for (Mipmap& mip : mMipmaps)
+        nif->readVectorOfRecords(mipmapsCount, mMipmaps);
-        {
+        const uint32_t numPixels = nif->get<uint32_t>();
-            nif->read(mip.mWidth);
-            nif->read(mip.mHeight);
-            nif->read(mip.mOffset);
-        }
-        uint32_t numPixels;
-        nif->read(numPixels);
         if (nif->getVersion() >= NIFStream::generateVersion(10, 4, 0, 2))
             nif->read(mNumFaces);
         nif->readVector(mData, numPixels * mNumFaces);
@@ -247,12 +288,14 @@ namespace Nif
 
     void NiVisData::read(NIFStream* nif)
     {
-        mKeys = std::make_shared<std::vector<std::pair<float, bool>>>(nif->get<uint32_t>());
+        const auto readPair = [](NIFStream& stream, std::pair<float, bool>& value) {
-        for (auto& [time, value] : *mKeys)
+            stream.read(value.first);
-        {
+            value.second = stream.get<uint8_t>() != 0;
-            nif->read(time);
+        };
-            value = nif->get<uint8_t>() != 0;
+
-        }
+        std::vector<std::pair<float, bool>> keys;
+        nif->readVectorOfRecords<uint32_t>(readPair, keys);
+        mKeys = std::make_shared<std::vector<std::pair<float, bool>>>(std::move(keys));
     }
 
     void NiSkinInstance::read(NIFStream* nif)
@@ -296,16 +339,17 @@ namespace Nif
         return partitions;
     }
 
+    void BSDismemberSkinInstance::BodyPart::read(NIFStream* nif)
+    {
+        nif->read(mFlags);
+        nif->read(mType);
+    }
+
     void BSDismemberSkinInstance::read(NIFStream* nif)
     {
         NiSkinInstance::read(nif);
 
-        mParts.resize(nif->get<uint32_t>());
+        nif->readVectorOfRecords<uint32_t>(mParts);
-        for (BodyPart& part : mParts)
-        {
-            nif->read(part.mFlags);
-            nif->read(part.mType);
-        }
     }
 
     void BSSkinInstance::read(NIFStream* nif)
@@ -339,8 +383,7 @@ namespace Nif
     {
         nif->read(mTransform);
 
-        uint32_t numBones;
+        const uint32_t numBones = nif->get<uint32_t>();
-        nif->read(numBones);
         bool hasVertexWeights = true;
         if (nif->getVersion() >= NIFFile::NIFVersion::VER_MW)
         {
@@ -351,25 +394,8 @@ namespace Nif
                 nif->read(hasVertexWeights);
         }
 
-        mBones.resize(numBones);
+        const ReadNiSkinDataBoneInfo readBoneInfo{ .mHasVertexWeights = hasVertexWeights };
-        for (BoneInfo& bi : mBones)
+        nif->readVectorOfRecords(numBones, readBoneInfo, mBones);
-        {
-            nif->read(bi.mTransform);
-            nif->read(bi.mBoundSphere);
-
-            uint16_t numVertices;
-            nif->read(numVertices);
-
-            if (!hasVertexWeights)
-                continue;
-
-            bi.mWeights.resize(numVertices);
-            for (auto& [vertex, weight] : bi.mWeights)
-            {
-                nif->read(vertex);
-                nif->read(weight);
-            }
-        }
     }
 
     void NiSkinData::post(Reader& nif)
@@ -467,18 +493,12 @@ namespace Nif
 
     void NiMorphData::read(NIFStream* nif)
     {
-        uint32_t numMorphs, numVerts;
+        const uint32_t numMorphs = nif->get<uint32_t>();
-        nif->read(numMorphs);
+        const uint32_t numVerts = nif->get<uint32_t>();
-        nif->read(numVerts);
         nif->read(mRelativeTargets);
 
-        mMorphs.resize(numMorphs);
+        const ReadNiMorphDataMorphData readMorph{ .mNumVerts = numVerts };
-        for (MorphData& morph : mMorphs)
+        nif->readVectorOfRecords(numMorphs, readMorph, mMorphs);
-        {
-            morph.mKeyFrames = std::make_shared<FloatKeyMap>();
-            morph.mKeyFrames->read(nif, /*morph*/ true);
-            nif->readVector(morph.mVertices, numVerts);
-        }
     }
 
     void NiKeyframeData::read(NIFStream* nif)

components/nif/data.hpp

@@ -169,8 +169,11 @@ namespace Nif
     {
         struct Mipmap
         {
-            uint32_t mWidth, mHeight;
+            uint32_t mWidth;
+            uint32_t mHeight;
             uint32_t mOffset;
+
+            void read(NIFStream* nif);
         };
 
         NiPixelFormat mPixelFormat;
@@ -218,6 +221,8 @@ namespace Nif
         {
             uint16_t mFlags;
             uint16_t mType;
+
+            void read(NIFStream* nif);
         };
 
         std::vector<BodyPart> mParts;

components/nif/extra.cpp

@@ -2,7 +2,6 @@
 
 namespace Nif
 {
-
     void NiExtraData::read(NIFStream* nif)
     {
         Extra::read(nif);
@@ -17,18 +16,17 @@ namespace Nif
         nif->getSizedStrings(mData, nif->get<uint32_t>());
     }
 
+    void NiTextKeyExtraData::TextKey::read(NIFStream* nif)
+    {
+        nif->read(mTime);
+        nif->read(mText);
+    }
+
     void NiTextKeyExtraData::read(NIFStream* nif)
     {
         Extra::read(nif);
 
-        uint32_t numKeys;
+        nif->readVectorOfRecords<uint32_t>(mList);
-        nif->read(numKeys);
-        mList.resize(numKeys);
-        for (TextKey& key : mList)
-        {
-            nif->read(key.mTime);
-            nif->read(key.mText);
-        }
     }
 
     void NiVertWeightsExtraData::read(NIFStream* nif)
@@ -66,20 +64,11 @@ namespace Nif
     {
         Extra::read(nif);
 
-        uint32_t num;
+        const uint32_t num = nif->get<uint32_t>();
-        nif->read(num);
         if (nif->getBethVersion() <= NIFFile::BethVersion::BETHVER_FO3)
-        {
+            nif->readVectorOfRecords(num, mLegacyMarkers);
-            mLegacyMarkers.resize(num);
-            for (auto& marker : mLegacyMarkers)
-                marker.read(nif);
-        }
         else
-        {
+            nif->readVectorOfRecords(num, mMarkers);
-            mMarkers.resize(num);
-            for (auto& marker : mMarkers)
-                marker.read(nif);
-        }
     }
 
     void BSInvMarker::read(NIFStream* nif)

components/nif/extra.hpp

@@ -77,6 +77,8 @@ namespace Nif
         {
             float mTime;
             std::string mText;
+
+            void read(NIFStream* nif);
         };
         std::vector<TextKey> mList;
 

components/nif/niffile.cpp

@@ -600,7 +600,9 @@ namespace Nif
         if (hasUserVersion)
             nif.read(mUserVersion);
 
-        mRecords.resize(nif.get<std::uint32_t>());
+        const std::uint32_t recordsCount = nif.get<std::uint32_t>();
+
+        mRecords.reserve(recordsCount);
 
         // Bethesda stream header
         {
@@ -646,14 +648,14 @@ namespace Nif
             else
             {
                 nif.getSizedStrings(recTypes, nif.get<std::uint16_t>());
-                nif.readVector(recTypeIndices, mRecords.size());
+                nif.readVector(recTypeIndices, recordsCount);
             }
         }
 
         if (hasRecordSizes) // Record sizes
         {
             std::vector<std::uint32_t> recSizes; // Currently unused
-            nif.readVector(recSizes, mRecords.size());
+            nif.readVector(recSizes, recordsCount);
         }
 
         if (hasStringTable)
@@ -670,11 +672,11 @@ namespace Nif
             nif.readVector(groups, nif.get<std::uint32_t>());
         }
 
-        for (std::size_t i = 0; i < mRecords.size(); i++)
+        for (std::size_t i = 0; i < recordsCount; i++)
         {
             std::unique_ptr<Record> r;
 
-            std::string rec = hasRecTypeListings ? recTypes[recTypeIndices[i]] : nif.get<std::string>();
+            std::string rec = hasRecTypeListings ? recTypes.at(recTypeIndices[i]) : nif.get<std::string>();
             if (rec.empty())
             {
                 std::stringstream error;
@@ -701,22 +703,23 @@ namespace Nif
             r->recName = std::move(rec);
             r->recIndex = static_cast<unsigned>(i);
             r->read(&nif);
-            mRecords[i] = std::move(r);
+            mRecords.push_back(std::move(r));
         }
 
         // Determine which records are roots
-        mRoots.resize(nif.get<uint32_t>());
+        const std::uint32_t rootsCount = nif.get<uint32_t>();
-        for (std::size_t i = 0; i < mRoots.size(); i++)
+        mRoots.reserve(rootsCount);
+        for (std::size_t i = 0; i < rootsCount; i++)
         {
             std::int32_t idx;
             nif.read(idx);
             if (idx >= 0 && static_cast<std::size_t>(idx) < mRecords.size())
             {
-                mRoots[i] = mRecords[idx].get();
+                mRoots.push_back(mRecords[idx].get());
             }
             else
             {
-                mRoots[i] = nullptr;
+                mRoots.push_back(nullptr);
                 Log(Debug::Warning) << "NIFFile Warning: Root " << i + 1 << " does not point to a record: index " << idx
                                     << ". File: " << mFilename;
             }

components/nif/nifkey.hpp

@@ -76,8 +76,7 @@ namespace Nif
                 }
             }
 
-            uint32_t count;
+            const uint32_t count = nif->get<uint32_t>();
-            nif->read(count);
 
             if (count == 0 && !morph)
                 return;
@@ -86,42 +85,21 @@ namespace Nif
 
             mKeys.reserve(count);
 
-            KeyType key = {};
-
             if (mInterpolationType == InterpolationType_Linear || mInterpolationType == InterpolationType_Constant)
             {
-                for (size_t i = 0; i < count; i++)
+                nif->readVectorOfRecords(count, readValuePair, mKeys);
-                {
-                    float time;
-                    nif->read(time);
-                    readValue(*nif, key);
-                    mKeys.emplace_back(time, key);
-                }
             }
             else if (mInterpolationType == InterpolationType_Quadratic)
             {
-                for (size_t i = 0; i < count; i++)
+                nif->readVectorOfRecords(count, readQuadraticPair, mKeys);
-                {
-                    float time;
-                    nif->read(time);
-                    readQuadratic(*nif, key);
-                    mKeys.emplace_back(time, key);
-                }
             }
             else if (mInterpolationType == InterpolationType_TCB)
             {
-                std::vector<TCBKey<T>> tcbKeys(count);
+                std::vector<TCBKey<T>> tcbKeys;
-                for (TCBKey<T>& tcbKey : tcbKeys)
+                nif->readVectorOfRecords(count, readTCBKey, tcbKeys);
-                {
-                    nif->read(tcbKey.mTime);
-                    tcbKey.mValue = ((*nif).*getValue)();
-                    nif->read(tcbKey.mTension);
-                    nif->read(tcbKey.mContinuity);
-                    nif->read(tcbKey.mBias);
-                }
                 generateTCBTangents(tcbKeys);
                 for (TCBKey<T>& tcbKey : tcbKeys)
-                    mKeys.emplace_back(std::move(tcbKey.mTime),
+                    mKeys.emplace_back(tcbKey.mTime,
                         KeyType{ std::move(tcbKey.mValue), std::move(tcbKey.mInTan), std::move(tcbKey.mOutTan) });
             }
             else if (mInterpolationType == InterpolationType_XYZ)
@@ -143,17 +121,42 @@ namespace Nif
         }
 
     private:
-        static void readValue(NIFStream& nif, KeyT<T>& key) { key.mValue = (nif.*getValue)(); }
+        static void readValue(NIFStream& nif, KeyType& key) { key.mValue = (nif.*getValue)(); }
 
-        template <typename U>
+        static void readValuePair(NIFStream& nif, std::pair<float, KeyType>& value)
-        static void readQuadratic(NIFStream& nif, KeyT<U>& key)
+        {
+            nif.read(value.first);
+            readValue(nif, value.second);
+        }
+
+        static void readQuadratic(NIFStream& nif, KeyType& key)
+        {
+            if constexpr (std::is_same_v<T, osg::Quat>)
+            {
+                readValue(nif, key);
+            }
+            else
             {
                 readValue(nif, key);
                 key.mInTan = (nif.*getValue)();
                 key.mOutTan = (nif.*getValue)();
             }
+        }
 
-        static void readQuadratic(NIFStream& nif, KeyT<osg::Quat>& key) { readValue(nif, key); }
+        static void readQuadraticPair(NIFStream& nif, std::pair<float, KeyType>& value)
+        {
+            nif.read(value.first);
+            readQuadratic(nif, value.second);
+        }
+
+        static void readTCBKey(NIFStream& nif, TCBKey<T>& value)
+        {
+            nif.read(value.mTime);
+            value.mValue = (nif.*getValue)();
+            nif.read(value.mTension);
+            nif.read(value.mContinuity);
+            nif.read(value.mBias);
+        }
 
         template <typename U>
         static void generateTCBTangents(std::vector<TCBKey<U>>& keys)
@@ -192,6 +195,16 @@ namespace Nif
         }
     };
 
+    template <class Key, class Value>
+    void readKeyMapPair(NIFStream& stream, std::pair<Key, KeyT<Value>>& value);
+
+    template <>
+    inline void readKeyMapPair(NIFStream& stream, std::pair<float, KeyT<bool>>& value)
+    {
+        stream.read(value.first);
+        value.second.mValue = stream.get<uint8_t>() != 0;
+    }
+
     using FloatKeyMap = KeyMapT<float, &NIFStream::get<float>>;
     using Vector3KeyMap = KeyMapT<osg::Vec3f, &NIFStream::get<osg::Vec3f>>;
     using Vector4KeyMap = KeyMapT<osg::Vec4f, &NIFStream::get<osg::Vec4f>>;

components/nif/nifstream.cpp

@@ -1,6 +1,10 @@
 #include "nifstream.hpp"
 
+#include <cerrno>
+#include <format>
 #include <span>
+#include <stdexcept>
+#include <system_error>
 
 #include <components/toutf8/toutf8.hpp>
 
@@ -53,10 +57,12 @@ namespace Nif
 
     std::string NIFStream::getSizedString(size_t length)
     {
+        checkStreamSize(length);
         std::string str(length, '\0');
         mStream->read(str.data(), length);
-        if (mStream->bad())
+        if (mStream->fail())
-            throw std::runtime_error("Failed to read sized string of " + std::to_string(length) + " chars");
+            throw std::runtime_error(std::format(
+                "Failed to read sized string of {} chars: {}", length, std::generic_category().message(errno)));
         size_t end = str.find('\0');
         if (end != std::string::npos)
             str.erase(end);
@@ -67,27 +73,31 @@ namespace Nif
 
     void NIFStream::getSizedStrings(std::vector<std::string>& vec, size_t size)
     {
-        vec.resize(size);
+        vec.clear();
-        for (size_t i = 0; i < vec.size(); i++)
+        vec.reserve(size);
-            vec[i] = getSizedString();
+        for (size_t i = 0; i < size; i++)
+            vec.push_back(getSizedString());
     }
 
     std::string NIFStream::getVersionString()
     {
         std::string result;
         std::getline(*mStream, result);
-        if (mStream->bad())
+        if (mStream->fail())
-            throw std::runtime_error("Failed to read version string");
+            throw std::runtime_error(
+                std::format("Failed to read version string: {}", std::generic_category().message(errno)));
         return result;
     }
 
     std::string NIFStream::getStringPalette()
     {
         size_t size = get<uint32_t>();
+        checkStreamSize(size);
         std::string str(size, '\0');
         mStream->read(str.data(), size);
-        if (mStream->bad())
+        if (mStream->fail())
-            throw std::runtime_error("Failed to read string palette of " + std::to_string(size) + " chars");
+            throw std::runtime_error(std::format(
+                "Failed to read string palette of {} chars: {}", size, std::generic_category().message(errno)));
         return str;
     }
 
@@ -228,6 +238,7 @@ namespace Nif
     {
         if (getVersion() < generateVersion(4, 1, 0, 0))
         {
+            checkStreamSize(size * sizeof(int32_t));
             std::vector<int32_t> buf(size);
             read(buf.data(), size);
             for (size_t i = 0; i < size; ++i)
@@ -235,6 +246,7 @@ namespace Nif
         }
         else
         {
+            checkStreamSize(size * sizeof(int8_t));
             std::vector<int8_t> buf(size);
             read(buf.data(), size);
             for (size_t i = 0; i < size; ++i)
@@ -257,4 +269,9 @@ namespace Nif
         }
     }
 
+    void NIFStream::checkStreamSize(std::size_t size)
+    {
+        if (size > mStreamSize)
+            throw std::runtime_error(std::format("Trying to read more than stream size: {} max={}", size, mStreamSize));
+    }
 }

components/nif/nifstream.hpp

@@ -5,14 +5,18 @@
 
 #include <array>
 #include <cassert>
+#include <cerrno>
+#include <format>
 #include <istream>
 #include <stdexcept>
 #include <stdint.h>
 #include <string>
+#include <system_error>
 #include <type_traits>
 #include <vector>
 
 #include <components/files/istreamptr.hpp>
+#include <components/files/utils.hpp>
 #include <components/misc/endianness.hpp>
 #include <components/misc/float16.hpp>
 
@@ -40,9 +44,9 @@ namespace Nif
             std::is_arithmetic_v<T> || std::is_same_v<T, Misc::float16_t>, "Buffer element type is not arithmetic");
         static_assert(!std::is_same_v<T, bool>, "Buffer element type is boolean");
         pIStream->read((char*)dest, numInstances * sizeof(T));
-        if (pIStream->bad())
+        if (pIStream->fail())
-            throw std::runtime_error("Failed to read typed (" + std::string(typeid(T).name()) + ") buffer of "
+            throw std::runtime_error(std::format("Failed to read typed ({}) dynamic buffer of {} instances: {}",
-                + std::to_string(numInstances) + " instances");
+                typeid(T).name(), numInstances, std::generic_category().message(errno)));
         if constexpr (Misc::IS_BIG_ENDIAN)
             for (std::size_t i = 0; i < numInstances; i++)
                 Misc::swapEndiannessInplace(dest[i]);
@@ -61,20 +65,26 @@ namespace Nif
             std::is_arithmetic_v<T> || std::is_same_v<T, Misc::float16_t>, "Buffer element type is not arithmetic");
         static_assert(!std::is_same_v<T, bool>, "Buffer element type is boolean");
         pIStream->read((char*)dest, numInstances * sizeof(T));
-        if (pIStream->bad())
+        if (pIStream->fail())
-            throw std::runtime_error("Failed to read typed (" + std::string(typeid(T).name()) + ") dynamic buffer of "
+            throw std::runtime_error(std::format("Failed to read typed ({}) dynamic buffer of {} instances: {}",
-                + std::to_string(numInstances) + " instances");
+                typeid(T).name(), numInstances, std::generic_category().message(errno)));
         if constexpr (Misc::IS_BIG_ENDIAN)
             for (std::size_t i = 0; i < numInstances; i++)
                 Misc::swapEndiannessInplace(dest[i]);
     }
 
+    class NIFStream;
+
+    template <class T>
+    void readRecord(NIFStream& stream, T& value);
+
     class NIFStream
     {
         const Reader& mReader;
         Files::IStreamPtr mStream;
         const ToUTF8::StatelessUtf8Encoder* mEncoder;
         std::string mBuffer;
+        std::size_t mStreamSize;
 
     public:
         explicit NIFStream(
@@ -82,6 +92,7 @@ namespace Nif
             : mReader(reader)
             , mStream(std::move(stream))
             , mEncoder(encoder)
+            , mStreamSize(static_cast<std::size_t>(Files::getStreamSizeLeft(*mStream)))
         {
         }
 
@@ -126,6 +137,9 @@ namespace Nif
         {
             if (size == 0)
                 return;
+
+            checkStreamSize(size * sizeof(T));
+
             vec.resize(size);
             read(vec.data(), size);
         }
@@ -156,8 +170,48 @@ namespace Nif
 
         /// Read a sequence of null-terminated strings
         std::string getStringPalette();
+
+        template <class Count, class T, class Read>
+        void readVectorOfRecords(Count count, Read&& read, std::vector<T>& values)
+        {
+            values.clear();
+            values.reserve(count);
+            for (Count i = 0; i < count; ++i)
+            {
+                T value;
+                read(*this, value);
+                values.push_back(std::move(value));
+            }
+        }
+
+        template <class Count, class T, class Read>
+        void readVectorOfRecords(Read&& read, std::vector<T>& values)
+        {
+            readVectorOfRecords(get<Count>(), std::forward<Read>(read), values);
+        }
+
+        template <class Count, class T>
+        void readVectorOfRecords(Count count, std::vector<T>& values)
+        {
+            readVectorOfRecords(count, readRecord<T>, values);
+        }
+
+        template <class Count, class T>
+        void readVectorOfRecords(std::vector<T>& values)
+        {
+            readVectorOfRecords<Count>(readRecord<T>, values);
+        }
+
+    private:
+        void checkStreamSize(std::size_t size);
     };
 
+    template <class T>
+    void readRecord(NIFStream& stream, T& value)
+    {
+        value.read(&stream);
+    }
+
     template <>
     void NIFStream::read<osg::Vec2f>(osg::Vec2f& vec);
     template <>

components/nif/node.cpp

@@ -105,9 +105,7 @@ namespace Nif
             }
             case UNION_BV:
             {
-                mChildren.resize(nif->get<uint32_t>());
+                nif->readVectorOfRecords<uint32_t>(mChildren);
-                for (BoundingVolume& child : mChildren)
-                    child.read(nif);
                 break;
             }
             case HALFSPACE_BV:
@@ -426,6 +424,12 @@ namespace Nif
         nif->read(mInitialIndex);
     }
 
+    void NiLODNode::LODRange::read(NIFStream* nif)
+    {
+        nif->read(mMinRange);
+        nif->read(mMaxRange);
+    }
+
     void NiLODNode::read(NIFStream* nif)
     {
         NiSwitchNode::read(nif);
@@ -439,12 +443,7 @@ namespace Nif
         if (nif->getVersion() >= NIFFile::NIFVersion::VER_MW)
             nif->read(mLODCenter);
 
-        mLODLevels.resize(nif->get<uint32_t>());
+        nif->readVectorOfRecords<uint32_t>(mLODLevels);
-        for (LODRange& level : mLODLevels)
-        {
-            nif->read(level.mMinRange);
-            nif->read(level.mMaxRange);
-        }
     }
 
     void NiFltAnimationNode::read(NIFStream* nif)

components/nif/node.hpp

@@ -232,6 +232,8 @@ namespace Nif
         {
             float mMinRange;
             float mMaxRange;
+
+            void read(NIFStream* nif);
         };
 
         osg::Vec3f mLODCenter;

components/nif/particle.cpp

@@ -225,11 +225,7 @@ namespace Nif
 
         bool hasData = nif->getBethVersion() < NIFFile::BethVersion::BETHVER_FO3;
         if (hasData)
-        {
+            nif->readVectorOfRecords(mNumVertices, mParticles);
-            mParticles.resize(mNumVertices);
-            for (NiParticleInfo& info : mParticles)
-                info.read(nif);
-        }
 
         if (nif->getBethVersion() > NIFFile::BethVersion::BETHVER_FO4)
             nif->skip(12); // Unknown
@@ -680,12 +676,7 @@ namespace Nif
         mFloatKeyList = std::make_shared<FloatKeyMap>();
         mFloatKeyList->read(nif);
         mVisKeyList = std::make_shared<BoolKeyMap>();
-        mVisKeyList->mKeys.resize(nif->get<uint32_t>());
+        nif->readVectorOfRecords<uint32_t>(readKeyMapPair<float, bool>, mVisKeyList->mKeys);
-        for (auto& [time, key] : mVisKeyList->mKeys)
-        {
-            nif->read(time);
-            key.mValue = nif->get<uint8_t>() != 0;
-        }
     }
 
     void NiPSysCollider::read(NIFStream* nif)

components/nif/physics.cpp

@@ -555,11 +555,7 @@ namespace Nif
         if (nif->getVersion() >= NIFStream::generateVersion(10, 1, 0, 0))
             nif->read(mScale);
         readRecordList(nif, mData);
-        uint32_t numFilters;
+        nif->readVectorOfRecords<uint32_t>(mHavokFilters);
-        nif->read(numFilters);
-        mHavokFilters.resize(numFilters);
-        for (HavokFilter& filter : mHavokFilters)
-            filter.read(nif);
     }
 
     void bhkNiTriStripsShape::post(Reader& nif)

components/nif/property.cpp

@@ -73,10 +73,12 @@ namespace Nif
         if (nif->getVersion() <= NIFStream::generateVersion(20, 1, 0, 1))
             mApplyMode = static_cast<ApplyMode>(nif->get<uint32_t>());
 
-        mTextures.resize(nif->get<uint32_t>());
+        const uint32_t texturesSize = nif->get<uint32_t>();
-        for (size_t i = 0; i < mTextures.size(); i++)
+
+        mTextures.reserve(texturesSize);
+        for (size_t i = 0; i < texturesSize; i++)
         {
-            mTextures[i].read(nif);
+            mTextures.emplace_back().read(nif);
 
             if (i == 5 && mTextures[5].mEnabled)
             {
@@ -89,13 +91,16 @@ namespace Nif
 
         if (nif->getVersion() >= NIFStream::generateVersion(10, 0, 1, 0))
         {
-            mShaderTextures.resize(nif->get<uint32_t>());
+            const uint32_t sharedTexturesSize = nif->get<uint32_t>();
-            mShaderIds.resize(mShaderTextures.size());
+            mShaderTextures.reserve(sharedTexturesSize);
-            for (size_t i = 0; i < mShaderTextures.size(); i++)
+            mShaderIds.reserve(sharedTexturesSize);
+            for (size_t i = 0; i < sharedTexturesSize; i++)
             {
-                mShaderTextures[i].read(nif);
+                mShaderTextures.emplace_back().read(nif);
+                uint32_t id = 0;
                 if (mShaderTextures[i].mEnabled)
-                    nif->read(mShaderIds[i]);
+                    nif->read(id);
+                mShaderIds.push_back(id);
             }
         }
     }
@@ -346,9 +351,10 @@ namespace Nif
                     mTranslucency.read(nif);
                 if (nif->get<uint8_t>() != 0)
                 {
-                    mTextureArrays.resize(nif->get<uint32_t>());
+                    const uint32_t size = nif->get<uint32_t>();
-                    for (std::vector<std::string>& textureArray : mTextureArrays)
+                    mTextureArrays.reserve(size);
-                        nif->getSizedStrings(textureArray, nif->get<uint32_t>());
+                    for (uint32_t i = 0; i < size; ++i)
+                        nif->getSizedStrings(mTextureArrays.emplace_back(), nif->get<uint32_t>());
                 }
             }
             if (nif->getBethVersion() >= NIFFile::BethVersion::BETHVER_STF)

components/nif/recordptr.hpp

@@ -1,6 +1,9 @@
 #ifndef OPENMW_COMPONENTS_NIF_RECORDPTR_HPP
 #define OPENMW_COMPONENTS_NIF_RECORDPTR_HPP
 
+#include <format>
+#include <stdexcept>
+#include <typeinfo>
 #include <vector>
 
 #include "niffile.hpp"
@@ -16,20 +19,40 @@ namespace Nif
     template <class X>
     class RecordPtrT
     {
+#ifndef NDEBUG
+        enum class State
+        {
+            Index,
+            Ptr,
+        };
+
+        State mState;
+#endif
+
         union
         {
-            intptr_t index;
+            intptr_t mIndex;
             X* mPtr;
         };
 
     public:
         RecordPtrT()
-            : index(-2)
+            :
+#ifndef NDEBUG
+            mState(State::Index)
+            ,
+#endif
+            mIndex(-2)
         {
         }
 
         RecordPtrT(X* ptr)
-            : mPtr(ptr)
+            :
+#ifndef NDEBUG
+            mState(State::Ptr)
+            ,
+#endif
+            mPtr(ptr)
         {
         }
 
@@ -37,48 +60,94 @@ namespace Nif
         void read(NIFStream* nif)
         {
             // Can only read the index once
-            assert(index == -2);
+#ifndef NDEBUG
+            assert(mState == State::Index);
+#endif
+            assert(mIndex == -2);
 
             // Store the index for later
-            index = nif->get<int32_t>();
+            const int32_t index = nif->get<int32_t>();
-            assert(index >= -1);
+            if (index < -1)
+                throw std::runtime_error(std::format("Invalid index: {}", index));
+
+            mIndex = index;
         }
 
         /// Resolve index to pointer
         void post(Reader& nif)
         {
-            if (index < 0)
+#ifndef NDEBUG
+            assert(mState == State::Index);
+#endif
+
+            if (mIndex < 0)
                 mPtr = nullptr;
             else
             {
-                Record* r = nif.getRecord(index);
+                Record* const r = nif.getRecord(mIndex);
-                // And cast it
+                if (r == nullptr)
-                mPtr = dynamic_cast<X*>(r);
+                    throw std::runtime_error(std::format("Record at {} is nullptr", mIndex));
-                assert(mPtr != nullptr);
+
+                X* const ptr = dynamic_cast<X*>(r);
+                if (ptr == nullptr)
+                    throw std::runtime_error(std::format("Failed to cast record pointer to {}", typeid(X).name()));
+
+                mPtr = ptr;
             }
+
+#ifndef NDEBUG
+            mState = State::Ptr;
+#endif
         }
 
         /// Look up the actual object from the index
         const X* getPtr() const
         {
+#ifndef NDEBUG
+            assert(mState == State::Ptr);
+#endif
             assert(mPtr != nullptr);
             return mPtr;
         }
+
         X* getPtr()
         {
+#ifndef NDEBUG
+            assert(mState == State::Ptr);
+#endif
             assert(mPtr != nullptr);
             return mPtr;
         }
 
-        const X& get() const { return *getPtr(); }
+        const X& get() const
-        X& get() { return *getPtr(); }
+        {
+            return *getPtr();
+        }
+
+        X& get()
+        {
+            return *getPtr();
+        }
 
         /// Syntactic sugar
-        const X* operator->() const { return getPtr(); }
+        const X* operator->() const
-        X* operator->() { return getPtr(); }
+        {
+            return getPtr();
+        }
+
+        X* operator->()
+        {
+            return getPtr();
+        }
 
         /// Pointers are allowed to be empty
-        bool empty() const { return mPtr == nullptr; }
+        bool empty() const
+        {
+#ifndef NDEBUG
+            assert(mState == State::Ptr);
+#endif
+            return mPtr == nullptr;
+        }
     };
 
     /** A list of references to other records. These are read as a list,
@@ -91,16 +160,7 @@ namespace Nif
     template <class T>
     void readRecordList(NIFStream* nif, RecordListT<T>& list)
     {
-        const std::uint32_t length = nif->get<std::uint32_t>();
+        nif->readVectorOfRecords<uint32_t>(list);
-
-        // No reasonable list can hit this generous limit
-        if (length >= (1 << 24))
-            throw std::runtime_error("Record list too long: " + std::to_string(length));
-
-        list.resize(length);
-
-        for (auto& value : list)
-            value.read(nif);
     }
 
     template <class T>