refactor: migrate NBT storage to unique_ptr, remove -fpermissive

CompoundTag/ListTag now use unique_ptr internally, fixing multiple memory leaks in getCompound/getList/getAllTags and tag overwrite paths.
This commit is contained in:
MatthewBeshay 2026-03-31 13:26:12 +11:00
parent 07861307a2
commit 2a7e5dc1d4
8 changed files with 110 additions and 124 deletions

View file

@ -40,7 +40,6 @@ endif
add_project_arguments(global_cpp_defs, language: ['cpp', 'c'])
global_cpp_args = [
'-fpermissive',
'-Wshift-count-overflow',
'-pipe',
]

View file

@ -122,9 +122,9 @@ void FallingTile::tick() {
if (tileEntity != nullptr) {
CompoundTag* swap = new CompoundTag();
tileEntity->save(swap);
std::vector<Tag*>* allTags = tileData->getAllTags();
for (auto it = allTags->begin();
it != allTags->end(); ++it) {
std::vector<Tag*> allTags = tileData->getAllTags();
for (auto it = allTags.begin();
it != allTags.end(); ++it) {
Tag* tag = *it;
if (tag->getName().compare(L"x") == 0 ||
tag->getName().compare(L"y") == 0 ||
@ -132,7 +132,6 @@ void FallingTile::tick() {
continue;
swap->put(tag->getName(), tag->copy());
}
delete allTags;
tileEntity->load(swap);
tileEntity->setChanged();
}

View file

@ -133,12 +133,11 @@ std::shared_ptr<Entity> BaseMobSpawner::loadDataAndAddEntity(
CompoundTag* data = new CompoundTag();
entity->save(data);
std::vector<Tag*>* tags = getNextSpawnData()->tag->getAllTags();
for (auto it = tags->begin(); it != tags->end(); ++it) {
std::vector<Tag*> tags = getNextSpawnData()->tag->getAllTags();
for (auto it = tags.begin(); it != tags.end(); ++it) {
Tag* tag = *it;
data->put(tag->getName(), tag->copy());
}
delete tags;
entity->load(data);
if (entity->level != nullptr) entity->level->addEntity(entity);
@ -153,13 +152,12 @@ std::shared_ptr<Entity> BaseMobSpawner::loadDataAndAddEntity(
CompoundTag* mountData = new CompoundTag();
mount->save(mountData);
std::vector<Tag*>* ridingTags = ridingTag->getAllTags();
for (auto it = ridingTags->begin(); it != ridingTags->end();
std::vector<Tag*> ridingTags = ridingTag->getAllTags();
for (auto it = ridingTags.begin(); it != ridingTags.end();
++it) {
Tag* tag = *it;
mountData->put(tag->getName(), tag->copy());
}
delete ridingTags;
mount->load(mountData);
mount->moveTo(rider->x, rider->y, rider->z, rider->yRot,
rider->xRot);

View file

@ -127,8 +127,8 @@ CompoundTag *GameRules::createTag()
void GameRules::loadFromTag(CompoundTag *tag)
{
vector<Tag *> *allTags = tag->getAllTags();
for (auto it = allTags->begin(); it != allTags->end(); ++it)
vector<Tag *> allTags = tag->getAllTags();
for (auto it = allTags.begin(); it != allTags.end(); ++it)
{
Tag *ruleTag = *it;
std::wstring ruleName = ruleTag->getName();
@ -136,7 +136,6 @@ void GameRules::loadFromTag(CompoundTag *tag)
set(ruleName, value);
}
delete allTags;
}
// Need to delete returned vector.

View file

@ -252,8 +252,8 @@ void StructureFeature::restoreSavedData(Level* level) {
} else {
CompoundTag* fullTag = savedData->getFullTag();
std::vector<Tag*>* allTags = fullTag->getAllTags();
for (auto it = allTags->begin(); it != allTags->end(); ++it) {
std::vector<Tag*> allTags = fullTag->getAllTags();
for (auto it = allTags.begin(); it != allTags.end(); ++it) {
Tag* featureTag = *it;
if (featureTag->getId() == Tag::TAG_Compound) {
CompoundTag* ct = (CompoundTag*)featureTag;

View file

@ -134,10 +134,10 @@ void SavedDataStorage::loadAuxValues() {
dis.close();
Tag* tag;
std::vector<Tag*>* allTags = tags->getAllTags();
auto itEnd = allTags->end();
for (auto it = allTags->begin(); it != itEnd; it++) {
tag = *it; // tags->getAllTags()->at(i);
std::vector<Tag*> allTags = tags->getAllTags();
auto itEnd = allTags.end();
for (auto it = allTags.begin(); it != itEnd; it++) {
tag = *it;
if (dynamic_cast<ShortTag*>(tag) != nullptr) {
ShortTag* sTag = (ShortTag*)tag;
@ -146,7 +146,6 @@ void SavedDataStorage::loadAuxValues() {
usedAuxIds.insert(uaiMapType::value_type(id, val));
}
}
delete allTags;
}
}

View file

@ -11,21 +11,20 @@
#include "ByteArrayTag.h"
#include "IntArrayTag.h"
#include <memory>
#include <unordered_map>
class CompoundTag : public Tag {
private:
std::unordered_map<std::wstring, Tag*> tags;
std::unordered_map<std::wstring, std::unique_ptr<Tag>> tags;
public:
CompoundTag() : Tag(L"") {}
CompoundTag(const std::wstring& name) : Tag(name) {}
void write(DataOutput* dos) {
auto itEnd = tags.end();
for (std::unordered_map<std::wstring, Tag*>::iterator it = tags.begin();
it != itEnd; it++) {
Tag::writeNamedTag(it->second, dos);
for (auto& [key, value] : tags) {
Tag::writeNamedTag(value.get(), dos);
}
dos->writeByte(Tag::TAG_End);
}
@ -39,22 +38,19 @@ public:
return;
}
tags.clear();
Tag* tag;
while ((tag = Tag::readNamedTag(dis))->getId() != Tag::TAG_End) {
tags[tag->getName()] = tag;
for (;;) {
std::unique_ptr<Tag> tag(Tag::readNamedTag(dis));
if (tag->getId() == Tag::TAG_End) break;
auto name = tag->getName();
tags[name] = std::move(tag);
}
delete tag;
}
std::vector<Tag*>* getAllTags() // 4J - was collection
{
// 4J - was return tags.values();
std::vector<Tag*>* ret = new std::vector<Tag*>;
auto itEnd = tags.end();
for (std::unordered_map<std::wstring, Tag*>::iterator it = tags.begin();
it != itEnd; it++) {
ret->push_back(it->second);
std::vector<Tag*> getAllTags() {
std::vector<Tag*> ret;
ret.reserve(tags.size());
for (auto& [key, value] : tags) {
ret.push_back(value.get());
}
return ret;
}
@ -62,47 +58,49 @@ public:
uint8_t getId() { return TAG_Compound; }
void put(const std::wstring& name, Tag* tag) {
tags[name] = tag->setName(name);
tag->setName(name);
tags[name] = std::unique_ptr<Tag>(tag);
}
void putByte(const std::wstring& name, uint8_t value) {
tags[name] = (new ByteTag(name, value));
tags[name] = std::make_unique<ByteTag>(name, value);
}
void putShort(const std::wstring& name, short value) {
tags[name] = (new ShortTag(name, value));
tags[name] = std::make_unique<ShortTag>(name, value);
}
void putInt(const std::wstring& name, int value) {
tags[name] = (new IntTag(name, value));
tags[name] = std::make_unique<IntTag>(name, value);
}
void putLong(const std::wstring& name, int64_t value) {
tags[name] = (new LongTag(name, value));
tags[name] = std::make_unique<LongTag>(name, value);
}
void putFloat(const std::wstring& name, float value) {
tags[name] = (new FloatTag(name, value));
tags[name] = std::make_unique<FloatTag>(name, value);
}
void putDouble(const std::wstring& name, double value) {
tags[name] = (new DoubleTag(name, value));
tags[name] = std::make_unique<DoubleTag>(name, value);
}
void putString(const std::wstring& name, const std::wstring& value) {
tags[name] = (new StringTag(name, value));
tags[name] = std::make_unique<StringTag>(name, value);
}
void putByteArray(const std::wstring& name, std::vector<uint8_t>& value) {
tags[name] = (new ByteArrayTag(name, value));
tags[name] = std::make_unique<ByteArrayTag>(name, value);
}
void putIntArray(const std::wstring& name, std::vector<int>& value) {
tags[name] = (new IntArrayTag(name, value));
tags[name] = std::make_unique<IntArrayTag>(name, value);
}
void putCompound(const std::wstring& name, CompoundTag* value) {
tags[name] = value->setName(std::wstring(name));
value->setName(name);
tags[name] = std::unique_ptr<Tag>(value);
}
void putBoolean(const std::wstring& name, bool val) {
@ -111,7 +109,7 @@ public:
Tag* get(const std::wstring& name) {
auto it = tags.find(name);
if (it != tags.end()) return it->second;
if (it != tags.end()) return it->second.get();
return nullptr;
}
@ -120,72 +118,87 @@ public:
}
uint8_t getByte(const std::wstring& name) {
if (tags.find(name) == tags.end()) return (uint8_t)0;
return ((ByteTag*)tags[name])->data;
auto it = tags.find(name);
if (it == tags.end()) return 0;
return static_cast<ByteTag*>(it->second.get())->data;
}
short getShort(const std::wstring& name) {
if (tags.find(name) == tags.end()) return (short)0;
return ((ShortTag*)tags[name])->data;
auto it = tags.find(name);
if (it == tags.end()) return 0;
return static_cast<ShortTag*>(it->second.get())->data;
}
int getInt(const std::wstring& name) {
if (tags.find(name) == tags.end()) return (int)0;
return ((IntTag*)tags[name])->data;
auto it = tags.find(name);
if (it == tags.end()) return 0;
return static_cast<IntTag*>(it->second.get())->data;
}
int64_t getLong(const std::wstring& name) {
if (tags.find(name) == tags.end()) return (int64_t)0;
return ((LongTag*)tags[name])->data;
auto it = tags.find(name);
if (it == tags.end()) return 0;
return static_cast<LongTag*>(it->second.get())->data;
}
float getFloat(const std::wstring& name) {
if (tags.find(name) == tags.end()) return (float)0;
return ((FloatTag*)tags[name])->data;
auto it = tags.find(name);
if (it == tags.end()) return 0;
return static_cast<FloatTag*>(it->second.get())->data;
}
double getDouble(const std::wstring& name) {
if (tags.find(name) == tags.end()) return (double)0;
return ((DoubleTag*)tags[name])->data;
auto it = tags.find(name);
if (it == tags.end()) return 0;
return static_cast<DoubleTag*>(it->second.get())->data;
}
std::wstring getString(const std::wstring& name) {
if (tags.find(name) == tags.end()) return std::wstring(L"");
return ((StringTag*)tags[name])->data;
auto it = tags.find(name);
if (it == tags.end()) return std::wstring(L"");
return static_cast<StringTag*>(it->second.get())->data;
}
std::vector<uint8_t> getByteArray(const std::wstring& name) {
if (tags.find(name) == tags.end()) return std::vector<uint8_t>();
return ((ByteArrayTag*)tags[name])->data;
auto it = tags.find(name);
if (it == tags.end()) return std::vector<uint8_t>();
return static_cast<ByteArrayTag*>(it->second.get())->data;
}
std::vector<int> getIntArray(const std::wstring& name) {
if (tags.find(name) == tags.end()) return std::vector<int>();
return ((IntArrayTag*)tags[name])->data;
auto it = tags.find(name);
if (it == tags.end()) return std::vector<int>();
return static_cast<IntArrayTag*>(it->second.get())->data;
}
CompoundTag* getCompound(const std::wstring& name) {
if (tags.find(name) == tags.end()) return new CompoundTag(name);
return (CompoundTag*)tags[name];
auto it = tags.find(name);
if (it == tags.end()) {
auto [it2, inserted] = tags.emplace(name, std::make_unique<CompoundTag>(name));
return static_cast<CompoundTag*>(it2->second.get());
}
return static_cast<CompoundTag*>(it->second.get());
}
ListTag<Tag>* getList(const std::wstring& name) {
if (tags.find(name) == tags.end()) return new ListTag<Tag>(name);
return (ListTag<Tag>*)tags[name];
auto it = tags.find(name);
if (it == tags.end()) {
auto [it2, inserted] = tags.emplace(name, std::make_unique<ListTag<Tag>>(name));
return static_cast<ListTag<Tag>*>(it2->second.get());
}
return static_cast<ListTag<Tag>*>(it->second.get());
}
bool getBoolean(const std::wstring& string) { return getByte(string) != 0; }
void remove(const std::wstring& name) {
auto it = tags.find(name);
if (it != tags.end()) tags.erase(it);
// tags.remove(name);
tags.erase(name);
}
std::wstring toString() {
static const int bufSize = 32;
static wchar_t buf[bufSize];
swprintf(buf, bufSize, L"%d entries", tags.size());
swprintf(buf, bufSize, L"%zu entries", tags.size());
return std::wstring(buf);
}
@ -211,19 +224,12 @@ public:
bool isEmpty() { return tags.empty(); }
virtual ~CompoundTag() {
auto itEnd = tags.end();
for (auto it = tags.begin(); it != itEnd; it++) {
delete it->second;
}
}
virtual ~CompoundTag() = default;
Tag* copy() {
CompoundTag* tag = new CompoundTag(getName());
auto itEnd = tags.end();
for (auto it = tags.begin(); it != itEnd; it++) {
tag->put((wchar_t*)it->first.c_str(), it->second->copy());
for (auto& [key, value] : tags) {
tag->put(key, value->copy());
}
return tag;
}
@ -233,18 +239,14 @@ public:
CompoundTag* o = (CompoundTag*)obj;
if (tags.size() == o->tags.size()) {
bool equal = true;
auto itEnd = tags.end();
for (auto it = tags.begin(); it != itEnd; it++) {
auto itFind = o->tags.find(it->first);
for (auto& [key, value] : tags) {
auto itFind = o->tags.find(key);
if (itFind == o->tags.end() ||
!it->second->equals(itFind->second)) {
equal = false;
break;
!value->equals(itFind->second.get())) {
return false;
}
}
return equal;
// return tags.entrySet().equals(o.tags.entrySet());
return true;
}
}
return false;

View file

@ -1,12 +1,13 @@
#pragma once
#include "Tag.h"
#include <memory>
#include <vector>
template <class T>
class ListTag : public Tag {
private:
std::vector<Tag*> list;
std::vector<std::unique_ptr<Tag>> list;
uint8_t type;
public:
@ -15,15 +16,14 @@ public:
void write(DataOutput* dos) {
if (list.size() > 0)
type = (list[0])->getId();
type = list[0]->getId();
else
type = static_cast<uint8_t>(1);
dos->writeByte(type);
dos->writeInt((int)list.size());
auto itEnd = list.end();
for (auto it = list.begin(); it != itEnd; it++) (*it)->write(dos);
for (auto& tag : list) tag->write(dos);
}
void load(DataInput* dis, int tagDepth) {
@ -39,9 +39,9 @@ public:
list.clear();
for (int i = 0; i < size; i++) {
Tag* tag = Tag::newTag(type, L"");
std::unique_ptr<Tag> tag(Tag::newTag(type, L""));
tag->load(dis, tagDepth);
list.push_back(tag);
list.push_back(std::move(tag));
}
}
@ -49,7 +49,7 @@ public:
std::wstring toString() {
static wchar_t buf[64];
swprintf(buf, 64, L"%d entries of type %ls", list.size(),
swprintf(buf, 64, L"%zu entries of type %ls", list.size(),
Tag::getTagName(type));
return std::wstring(buf);
}
@ -62,9 +62,8 @@ public:
char* newPrefix = new char[strlen(prefix) + 4];
strcpy(newPrefix, prefix);
strcat(newPrefix, " ");
auto itEnd = list.end();
for (auto it = list.begin(); it != itEnd; it++) {
(*it)->print(newPrefix, out);
for (auto& tag : list) {
tag->print(newPrefix, out);
}
delete[] newPrefix;
out << prefix << "}" << std::endl;
@ -78,27 +77,20 @@ public:
// other items that also use list tags and require equality checks to
// work) considering we can't change the write/load functions.
tag->setName(L"");
list.push_back(tag);
list.push_back(std::unique_ptr<Tag>(tag));
}
T* get(int index) { return (T*)list[index]; }
T* get(int index) { return static_cast<T*>(list[index].get()); }
int size() { return (int)list.size(); }
virtual ~ListTag() {
auto itEnd = list.end();
for (auto it = list.begin(); it != itEnd; it++) {
delete *it;
}
}
virtual ~ListTag() = default;
virtual Tag* copy() {
ListTag<T>* res = new ListTag<T>(getName());
res->type = type;
auto itEnd = list.end();
for (auto it = list.begin(); it != itEnd; it++) {
T* copy = (T*)(*it)->copy();
res->list.push_back(copy);
for (auto& tag : list) {
res->list.push_back(std::unique_ptr<Tag>(tag->copy()));
}
return res;
}
@ -110,15 +102,13 @@ public:
bool equal = false;
if (list.size() == o->list.size()) {
equal = true;
auto itEnd = list.end();
// 4J Stu - Pretty inefficient method, but I think we can
// live with it give how often it will happen, and the small
// sizes of the data sets
for (auto it = list.begin(); it != itEnd; ++it) {
for (auto& tag : list) {
bool thisMatches = false;
for (auto it2 = o->list.begin(); it2 != o->list.end();
++it2) {
if ((*it)->equals(*it2)) {
for (auto& otherTag : o->list) {
if (tag->equals(otherTag.get())) {
thisMatches = true;
break;
}
@ -135,4 +125,4 @@ public:
}
return false;
}
};
};