mirror of
https://github.com/OpenMW/openmw.git
synced 2025-10-18 07:16:39 +00:00
Do not rely on std::string::reserve
Passing an object with capacity through std::ostringstream may change its capacity. Use malloc instead to make sure the memory is allocated.
This commit is contained in:
parent
c75aed5175
commit
ab4637f6c7
1 changed files with 36 additions and 11 deletions
|
@ -8,10 +8,14 @@
|
||||||
#include <gtest/gtest.h>
|
#include <gtest/gtest.h>
|
||||||
|
|
||||||
#include <cstdint>
|
#include <cstdint>
|
||||||
|
#include <cstdlib>
|
||||||
|
#include <cstring>
|
||||||
#include <filesystem>
|
#include <filesystem>
|
||||||
#include <format>
|
#include <format>
|
||||||
#include <fstream>
|
#include <fstream>
|
||||||
|
#include <memory>
|
||||||
#include <sstream>
|
#include <sstream>
|
||||||
|
#include <vector>
|
||||||
|
|
||||||
namespace Bsa
|
namespace Bsa
|
||||||
{
|
{
|
||||||
|
@ -19,6 +23,17 @@ namespace Bsa
|
||||||
{
|
{
|
||||||
using namespace ::testing;
|
using namespace ::testing;
|
||||||
|
|
||||||
|
struct Free
|
||||||
|
{
|
||||||
|
void operator()(void* ptr) const { std::free(ptr); }
|
||||||
|
};
|
||||||
|
|
||||||
|
struct Buffer
|
||||||
|
{
|
||||||
|
std::unique_ptr<char, Free> mData;
|
||||||
|
std::size_t mCapacity;
|
||||||
|
};
|
||||||
|
|
||||||
struct Header
|
struct Header
|
||||||
{
|
{
|
||||||
uint32_t mFormat;
|
uint32_t mFormat;
|
||||||
|
@ -69,13 +84,9 @@ namespace Bsa
|
||||||
std::format("{}.{}.bsa", testInfo->test_suite_name(), testInfo->name()));
|
std::format("{}.{}.bsa", testInfo->test_suite_name(), testInfo->name()));
|
||||||
}
|
}
|
||||||
|
|
||||||
std::string makeBsaBuffer(std::uint32_t fileSize, std::uint32_t fileOffset)
|
Buffer makeBsaBuffer(std::uint32_t fileSize, std::uint32_t fileOffset)
|
||||||
{
|
{
|
||||||
std::string buffer;
|
std::ostringstream stream;
|
||||||
|
|
||||||
buffer.reserve(static_cast<std::size_t>(fileSize) + static_cast<std::size_t>(fileOffset) + 34);
|
|
||||||
|
|
||||||
std::ostringstream stream(std::move(buffer));
|
|
||||||
|
|
||||||
const Header header{
|
const Header header{
|
||||||
.mFormat = static_cast<std::uint32_t>(BsaVersion::Uncompressed),
|
.mFormat = static_cast<std::uint32_t>(BsaVersion::Uncompressed),
|
||||||
|
@ -98,7 +109,17 @@ namespace Bsa
|
||||||
|
|
||||||
writeArchive(archive, stream);
|
writeArchive(archive, stream);
|
||||||
|
|
||||||
return std::move(stream).str();
|
const std::string data = std::move(stream).str();
|
||||||
|
|
||||||
|
const std::size_t capacity = static_cast<std::size_t>(fileSize) + static_cast<std::size_t>(fileOffset) + 34;
|
||||||
|
std::unique_ptr<char, Free> buffer(reinterpret_cast<char*>(std::malloc(capacity)));
|
||||||
|
|
||||||
|
if (buffer == nullptr)
|
||||||
|
throw std::bad_alloc();
|
||||||
|
|
||||||
|
std::memcpy(buffer.get(), data.data(), data.size());
|
||||||
|
|
||||||
|
return Buffer{ .mData = std::move(buffer), .mCapacity = capacity };
|
||||||
}
|
}
|
||||||
|
|
||||||
TEST(BSAFileTest, shouldHandleEmpty)
|
TEST(BSAFileTest, shouldHandleEmpty)
|
||||||
|
@ -266,11 +287,13 @@ namespace Bsa
|
||||||
TEST(BSAFileTest, shouldHandleSingleFileAtTheEndOfLargeFile)
|
TEST(BSAFileTest, shouldHandleSingleFileAtTheEndOfLargeFile)
|
||||||
{
|
{
|
||||||
constexpr std::uint32_t maxUInt32 = std::numeric_limits<uint32_t>::max();
|
constexpr std::uint32_t maxUInt32 = std::numeric_limits<uint32_t>::max();
|
||||||
const std::string buffer = makeBsaBuffer(maxUInt32, maxUInt32 - 34);
|
constexpr std::uint32_t fileSize = maxUInt32;
|
||||||
|
constexpr std::uint32_t fileOffset = maxUInt32 - 34;
|
||||||
|
const Buffer buffer = makeBsaBuffer(fileSize, fileOffset);
|
||||||
|
|
||||||
TestBSAFile file;
|
TestBSAFile file;
|
||||||
// Use capacity assuming we never read beyond small header.
|
// Use capacity assuming we never read beyond small header.
|
||||||
Files::IMemStream stream(buffer.data(), buffer.capacity());
|
Files::IMemStream stream(buffer.mData.get(), buffer.mCapacity);
|
||||||
file.readHeader(stream);
|
file.readHeader(stream);
|
||||||
|
|
||||||
std::vector<char> namesBuffer = { 'a', '\0' };
|
std::vector<char> namesBuffer = { 'a', '\0' };
|
||||||
|
@ -289,11 +312,13 @@ namespace Bsa
|
||||||
TEST(BSAFileTest, shouldThrowExceptionOnTooBigAbsoluteOffset)
|
TEST(BSAFileTest, shouldThrowExceptionOnTooBigAbsoluteOffset)
|
||||||
{
|
{
|
||||||
constexpr std::uint32_t maxUInt32 = std::numeric_limits<uint32_t>::max();
|
constexpr std::uint32_t maxUInt32 = std::numeric_limits<uint32_t>::max();
|
||||||
const std::string buffer = makeBsaBuffer(maxUInt32, maxUInt32 - 34 + 1);
|
constexpr std::uint32_t fileSize = maxUInt32;
|
||||||
|
constexpr std::uint32_t fileOffset = maxUInt32 - 34 + 1;
|
||||||
|
const Buffer buffer = makeBsaBuffer(fileSize, fileOffset);
|
||||||
|
|
||||||
TestBSAFile file;
|
TestBSAFile file;
|
||||||
// Use capacity assuming we never read beyond small header.
|
// Use capacity assuming we never read beyond small header.
|
||||||
Files::IMemStream stream(buffer.data(), buffer.capacity());
|
Files::IMemStream stream(buffer.mData.get(), buffer.mCapacity);
|
||||||
EXPECT_THROW(file.readHeader(stream), std::runtime_error);
|
EXPECT_THROW(file.readHeader(stream), std::runtime_error);
|
||||||
|
|
||||||
EXPECT_THAT(file.getList(), IsEmpty());
|
EXPECT_THAT(file.getList(), IsEmpty());
|
||||||
|
|
Loading…
Reference in a new issue