From 903ce446348061926329f144fc01fa178f1cc249 Mon Sep 17 00:00:00 2001 From: elsid Date: Wed, 5 May 2021 01:20:18 +0200 Subject: [PATCH 1/7] Revert "Various fixes for niftest" This reverts commit 87ada56edd9d5d5b0a9e6a8dcfd06eb09b161746. --- components/nif/niffile.cpp | 12 +++++------ components/nif/nifstream.cpp | 2 +- components/nif/nifstream.hpp | 41 ++++++++++++++++-------------------- 3 files changed, 25 insertions(+), 30 deletions(-) diff --git a/components/nif/niffile.cpp b/components/nif/niffile.cpp index 864a388e87..3e226b35e1 100644 --- a/components/nif/niffile.cpp +++ b/components/nif/niffile.cpp @@ -212,7 +212,7 @@ void NIFFile::parse(Files::IStreamPtr stream) userVer = nif.getUInt(); // Number of records - const std::size_t recNum = nif.getUInt(); + unsigned int recNum = nif.getUInt(); records.resize(recNum); // Bethesda stream header @@ -251,7 +251,7 @@ void NIFFile::parse(Files::IStreamPtr stream) std::vector recSizes; // Currently unused nif.getUInts(recSizes, recNum); } - const std::size_t stringNum = nif.getUInt(); + unsigned int stringNum = nif.getUInt(); nif.getUInt(); // Max string length if (stringNum) nif.getSizedStrings(strings, stringNum); @@ -264,7 +264,7 @@ void NIFFile::parse(Files::IStreamPtr stream) } const bool hasRecordSeparators = ver >= NIFStream::generateVersion(10,0,0,0) && ver < NIFStream::generateVersion(10,2,0,0); - for (std::size_t i = 0; i < recNum; i++) + for (unsigned int i = 0; i < recNum; i++) { Record *r = nullptr; @@ -308,14 +308,14 @@ void NIFFile::parse(Files::IStreamPtr stream) r->read(&nif); } - const std::size_t rootNum = nif.getUInt(); + unsigned int rootNum = nif.getUInt(); roots.resize(rootNum); //Determine which records are roots - for (std::size_t i = 0; i < rootNum; i++) + for (unsigned int i = 0; i < rootNum; i++) { int idx = nif.getInt(); - if (idx >= 0 && static_cast(idx) < records.size()) + if (idx >= 0 && idx < int(records.size())) { roots[i] = records[idx]; } diff --git a/components/nif/nifstream.cpp b/components/nif/nifstream.cpp index 6129a17394..07c9c917ce 100644 --- a/components/nif/nifstream.cpp +++ b/components/nif/nifstream.cpp @@ -7,7 +7,7 @@ namespace Nif osg::Quat NIFStream::getQuaternion() { float f[4]; - readLittleEndianBufferOfType<4, float>(inp, f); + readLittleEndianBufferOfType<4, float>(inp, (float*)&f); osg::Quat quat; quat.w() = f[0]; quat.x() = f[1]; diff --git a/components/nif/nifstream.hpp b/components/nif/nifstream.hpp index b6bf01ce58..4d221b8676 100644 --- a/components/nif/nifstream.hpp +++ b/components/nif/nifstream.hpp @@ -7,8 +7,6 @@ #include #include #include -#include -#include #include #include @@ -24,32 +22,31 @@ namespace Nif class NIFFile; -template inline void readLittleEndianBufferOfType(Files::IStreamPtr &pIStream, T* dest) +/* + readLittleEndianBufferOfType: This template should only be used with arithmetic types +*/ +template inline void readLittleEndianBufferOfType(Files::IStreamPtr &pIStream, T* dest) { - static_assert(std::is_arithmetic_v, "Buffer element type is not arithmetic"); pIStream->read((char*)dest, numInstances * sizeof(T)); - if (pIStream->bad()) - throw std::runtime_error("Failed to read little endian typed (" + std::string(typeid(T).name()) + ") buffer of " - + std::to_string(numInstances) + " instances"); if constexpr (Misc::IS_BIG_ENDIAN) - for (std::size_t i = 0; i < numInstances; i++) + for (uint32_t i = 0; i < numInstances; i++) Misc::swapEndiannessInplace(dest[i]); } -template inline void readLittleEndianDynamicBufferOfType(Files::IStreamPtr &pIStream, T* dest, std::size_t numInstances) +/* + readLittleEndianDynamicBufferOfType: This template should only be used with arithmetic types +*/ +template inline void readLittleEndianDynamicBufferOfType(Files::IStreamPtr &pIStream, T* dest, uint32_t numInstances) { - static_assert(std::is_arithmetic_v, "Buffer element type is not arithmetic"); pIStream->read((char*)dest, numInstances * sizeof(T)); - if (pIStream->bad()) - throw std::runtime_error("Failed to read little endian dynamic buffer of " + std::to_string(numInstances) + " instances"); if constexpr (Misc::IS_BIG_ENDIAN) - for (std::size_t i = 0; i < numInstances; i++) + for (uint32_t i = 0; i < numInstances; i++) Misc::swapEndiannessInplace(dest[i]); } template type inline readLittleEndianType(Files::IStreamPtr &pIStream) { type val; - readLittleEndianBufferOfType<1, type>(pIStream, &val); + readLittleEndianBufferOfType<1, type>(pIStream, (type*)&val); return val; } @@ -99,21 +96,21 @@ public: osg::Vec2f getVector2() { osg::Vec2f vec; - readLittleEndianBufferOfType<2,float>(inp, vec._v); + readLittleEndianBufferOfType<2,float>(inp, (float*)&vec._v[0]); return vec; } osg::Vec3f getVector3() { osg::Vec3f vec; - readLittleEndianBufferOfType<3, float>(inp, vec._v); + readLittleEndianBufferOfType<3, float>(inp, (float*)&vec._v[0]); return vec; } osg::Vec4f getVector4() { osg::Vec4f vec; - readLittleEndianBufferOfType<4, float>(inp, vec._v); + readLittleEndianBufferOfType<4, float>(inp, (float*)&vec._v[0]); return vec; } @@ -145,11 +142,11 @@ public: ///Read in a string of the given length std::string getSizedString(size_t length) { - std::string str(length, '\0'); + std::vector str(length + 1, 0); + inp->read(str.data(), length); - if (inp->bad()) - throw std::runtime_error("Failed to read sized string of " + std::to_string(length) + " chars"); - return str; + + return str.data(); } ///Read in a string of the length specified in the file std::string getSizedString() @@ -170,8 +167,6 @@ public: { std::string result; std::getline(*inp, result); - if (inp->bad()) - throw std::runtime_error("Failed to read version string"); return result; } From f4cfade14b8830d516bf45eb20523aa17960faa1 Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 3 May 2021 21:30:31 +0200 Subject: [PATCH 2/7] Use std::size_t to iterate --- components/nif/niffile.cpp | 10 +++++----- components/nif/nifstream.hpp | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/components/nif/niffile.cpp b/components/nif/niffile.cpp index 3e226b35e1..221f1d1f35 100644 --- a/components/nif/niffile.cpp +++ b/components/nif/niffile.cpp @@ -212,7 +212,7 @@ void NIFFile::parse(Files::IStreamPtr stream) userVer = nif.getUInt(); // Number of records - unsigned int recNum = nif.getUInt(); + const std::size_t recNum = nif.getUInt(); records.resize(recNum); // Bethesda stream header @@ -251,7 +251,7 @@ void NIFFile::parse(Files::IStreamPtr stream) std::vector recSizes; // Currently unused nif.getUInts(recSizes, recNum); } - unsigned int stringNum = nif.getUInt(); + const std::size_t stringNum = nif.getUInt(); nif.getUInt(); // Max string length if (stringNum) nif.getSizedStrings(strings, stringNum); @@ -264,7 +264,7 @@ void NIFFile::parse(Files::IStreamPtr stream) } const bool hasRecordSeparators = ver >= NIFStream::generateVersion(10,0,0,0) && ver < NIFStream::generateVersion(10,2,0,0); - for (unsigned int i = 0; i < recNum; i++) + for (std::size_t i = 0; i < recNum; i++) { Record *r = nullptr; @@ -308,11 +308,11 @@ void NIFFile::parse(Files::IStreamPtr stream) r->read(&nif); } - unsigned int rootNum = nif.getUInt(); + const std::size_t rootNum = nif.getUInt(); roots.resize(rootNum); //Determine which records are roots - for (unsigned int i = 0; i < rootNum; i++) + for (std::size_t i = 0; i < rootNum; i++) { int idx = nif.getInt(); if (idx >= 0 && idx < int(records.size())) diff --git a/components/nif/nifstream.hpp b/components/nif/nifstream.hpp index 4d221b8676..cee98dd831 100644 --- a/components/nif/nifstream.hpp +++ b/components/nif/nifstream.hpp @@ -25,22 +25,22 @@ class NIFFile; /* readLittleEndianBufferOfType: This template should only be used with arithmetic types */ -template inline void readLittleEndianBufferOfType(Files::IStreamPtr &pIStream, T* dest) +template inline void readLittleEndianBufferOfType(Files::IStreamPtr &pIStream, T* dest) { pIStream->read((char*)dest, numInstances * sizeof(T)); if constexpr (Misc::IS_BIG_ENDIAN) - for (uint32_t i = 0; i < numInstances; i++) + for (std::size_t i = 0; i < numInstances; i++) Misc::swapEndiannessInplace(dest[i]); } /* readLittleEndianDynamicBufferOfType: This template should only be used with arithmetic types */ -template inline void readLittleEndianDynamicBufferOfType(Files::IStreamPtr &pIStream, T* dest, uint32_t numInstances) +template inline void readLittleEndianDynamicBufferOfType(Files::IStreamPtr &pIStream, T* dest, std::size_t numInstances) { pIStream->read((char*)dest, numInstances * sizeof(T)); if constexpr (Misc::IS_BIG_ENDIAN) - for (uint32_t i = 0; i < numInstances; i++) + for (std::size_t i = 0; i < numInstances; i++) Misc::swapEndiannessInplace(dest[i]); } template type inline readLittleEndianType(Files::IStreamPtr &pIStream) From 3e4abb9f049c66c08c23797190b5c2bc44f2862e Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 3 May 2021 21:32:38 +0200 Subject: [PATCH 3/7] Avoid copy from vector to string Read directly into a string. --- components/nif/nifstream.hpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/components/nif/nifstream.hpp b/components/nif/nifstream.hpp index cee98dd831..1c4407afd9 100644 --- a/components/nif/nifstream.hpp +++ b/components/nif/nifstream.hpp @@ -142,11 +142,9 @@ public: ///Read in a string of the given length std::string getSizedString(size_t length) { - std::vector str(length + 1, 0); - + std::string str(length, '\0'); inp->read(str.data(), length); - - return str.data(); + return str; } ///Read in a string of the length specified in the file std::string getSizedString() From 1c08bc0b15ffc713018dd7de5aaa90dcf8f7e92f Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 3 May 2021 21:41:47 +0200 Subject: [PATCH 4/7] Handle std::istream bad state after read --- components/nif/nifstream.hpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/components/nif/nifstream.hpp b/components/nif/nifstream.hpp index 1c4407afd9..4d77b73262 100644 --- a/components/nif/nifstream.hpp +++ b/components/nif/nifstream.hpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -28,6 +29,9 @@ class NIFFile; template inline void readLittleEndianBufferOfType(Files::IStreamPtr &pIStream, T* dest) { pIStream->read((char*)dest, numInstances * sizeof(T)); + if (pIStream->bad()) + throw std::runtime_error("Failed to read little endian typed (" + std::string(typeid(T).name()) + ") buffer of " + + std::to_string(numInstances) + " instances"); if constexpr (Misc::IS_BIG_ENDIAN) for (std::size_t i = 0; i < numInstances; i++) Misc::swapEndiannessInplace(dest[i]); @@ -39,6 +43,8 @@ template inline void readLittleEndianBuff template inline void readLittleEndianDynamicBufferOfType(Files::IStreamPtr &pIStream, T* dest, std::size_t numInstances) { pIStream->read((char*)dest, numInstances * sizeof(T)); + if (pIStream->bad()) + throw std::runtime_error("Failed to read little endian dynamic buffer of " + std::to_string(numInstances) + " instances"); if constexpr (Misc::IS_BIG_ENDIAN) for (std::size_t i = 0; i < numInstances; i++) Misc::swapEndiannessInplace(dest[i]); @@ -144,6 +150,8 @@ public: { std::string str(length, '\0'); inp->read(str.data(), length); + if (inp->bad()) + throw std::runtime_error("Failed to read sized string of " + std::to_string(length) + " chars"); return str; } ///Read in a string of the length specified in the file @@ -165,6 +173,8 @@ public: { std::string result; std::getline(*inp, result); + if (inp->bad()) + throw std::runtime_error("Failed to read version string"); return result; } From bf2f15342bfee7e1bec19ff9abdb1999a4575f74 Mon Sep 17 00:00:00 2001 From: elsid Date: Tue, 4 May 2021 01:09:29 +0200 Subject: [PATCH 5/7] Compare positive int as std::size_t If records.size() is greater than max int comparison is invalid. --- components/nif/niffile.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/nif/niffile.cpp b/components/nif/niffile.cpp index 221f1d1f35..864a388e87 100644 --- a/components/nif/niffile.cpp +++ b/components/nif/niffile.cpp @@ -315,7 +315,7 @@ void NIFFile::parse(Files::IStreamPtr stream) for (std::size_t i = 0; i < rootNum; i++) { int idx = nif.getInt(); - if (idx >= 0 && idx < int(records.size())) + if (idx >= 0 && static_cast(idx) < records.size()) { roots[i] = records[idx]; } From d9e7c2fb42fb8e62323f609a1c4ac8f40b8a4a88 Mon Sep 17 00:00:00 2001 From: elsid Date: Tue, 4 May 2021 13:21:47 +0200 Subject: [PATCH 6/7] Replace comment by static assert --- components/nif/nifstream.hpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/components/nif/nifstream.hpp b/components/nif/nifstream.hpp index 4d77b73262..ad0ba5da8b 100644 --- a/components/nif/nifstream.hpp +++ b/components/nif/nifstream.hpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -23,11 +24,9 @@ namespace Nif class NIFFile; -/* - readLittleEndianBufferOfType: This template should only be used with arithmetic types -*/ template inline void readLittleEndianBufferOfType(Files::IStreamPtr &pIStream, T* dest) { + static_assert(std::is_arithmetic_v, "Buffer element type is not arithmetic"); pIStream->read((char*)dest, numInstances * sizeof(T)); if (pIStream->bad()) throw std::runtime_error("Failed to read little endian typed (" + std::string(typeid(T).name()) + ") buffer of " @@ -37,11 +36,9 @@ template inline void readLittleEndianBuff Misc::swapEndiannessInplace(dest[i]); } -/* - readLittleEndianDynamicBufferOfType: This template should only be used with arithmetic types -*/ template inline void readLittleEndianDynamicBufferOfType(Files::IStreamPtr &pIStream, T* dest, std::size_t numInstances) { + static_assert(std::is_arithmetic_v, "Buffer element type is not arithmetic"); pIStream->read((char*)dest, numInstances * sizeof(T)); if (pIStream->bad()) throw std::runtime_error("Failed to read little endian dynamic buffer of " + std::to_string(numInstances) + " instances"); From d5e28d7269e3ed33b43d56dbbc0b05eae6a00a79 Mon Sep 17 00:00:00 2001 From: elsid Date: Tue, 4 May 2021 13:18:44 +0200 Subject: [PATCH 7/7] Remove redundant casts --- components/nif/nifstream.cpp | 2 +- components/nif/nifstream.hpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/components/nif/nifstream.cpp b/components/nif/nifstream.cpp index 07c9c917ce..6129a17394 100644 --- a/components/nif/nifstream.cpp +++ b/components/nif/nifstream.cpp @@ -7,7 +7,7 @@ namespace Nif osg::Quat NIFStream::getQuaternion() { float f[4]; - readLittleEndianBufferOfType<4, float>(inp, (float*)&f); + readLittleEndianBufferOfType<4, float>(inp, f); osg::Quat quat; quat.w() = f[0]; quat.x() = f[1]; diff --git a/components/nif/nifstream.hpp b/components/nif/nifstream.hpp index ad0ba5da8b..b6bf01ce58 100644 --- a/components/nif/nifstream.hpp +++ b/components/nif/nifstream.hpp @@ -49,7 +49,7 @@ template inline void readLittleEndianDynamicBufferOfType(Files::ISt template type inline readLittleEndianType(Files::IStreamPtr &pIStream) { type val; - readLittleEndianBufferOfType<1, type>(pIStream, (type*)&val); + readLittleEndianBufferOfType<1, type>(pIStream, &val); return val; } @@ -99,21 +99,21 @@ public: osg::Vec2f getVector2() { osg::Vec2f vec; - readLittleEndianBufferOfType<2,float>(inp, (float*)&vec._v[0]); + readLittleEndianBufferOfType<2,float>(inp, vec._v); return vec; } osg::Vec3f getVector3() { osg::Vec3f vec; - readLittleEndianBufferOfType<3, float>(inp, (float*)&vec._v[0]); + readLittleEndianBufferOfType<3, float>(inp, vec._v); return vec; } osg::Vec4f getVector4() { osg::Vec4f vec; - readLittleEndianBufferOfType<4, float>(inp, (float*)&vec._v[0]); + readLittleEndianBufferOfType<4, float>(inp, vec._v); return vec; }