commit 892163e44aa57a8ae6cee0a45a887f36816a9a80
parent 3261187024a20d52cb8ee56cf7590e989be7c755
Author: cfillion <cfillion@users.noreply.github.com>
Date: Tue, 12 Jul 2016 18:24:01 -0400
fix a few possible memory leaks when loading repository indexes
Diffstat:
10 files changed, 48 insertions(+), 44 deletions(-)
diff --git a/src/index.cpp b/src/index.cpp
@@ -117,19 +117,21 @@ void Index::setName(const string &newName)
m_name = newName;
}
-void Index::addCategory(const Category *cat)
+bool Index::addCategory(const Category *cat)
{
if(cat->index() != this)
throw reapack_error("category belongs to another index");
if(cat->packages().empty())
- return;
+ return false;
m_catMap.insert({cat->name(), m_categories.size()});
m_categories.push_back(cat);
m_packages.insert(m_packages.end(),
cat->packages().begin(), cat->packages().end());
+
+ return true;
}
const Category *Index::category(const string &name) const
@@ -160,18 +162,17 @@ string Category::fullName() const
return m_index ? m_index->name() + "/" + m_name : m_name;
}
-void Category::addPackage(const Package *pkg)
+bool Category::addPackage(const Package *pkg)
{
if(pkg->category() != this)
throw reapack_error("package belongs to another category");
- if(pkg->type() == Package::UnknownType)
- return; // silently discard unknown package types
- else if(pkg->versions().empty())
- return;
+ if(pkg->type() == Package::UnknownType || pkg->versions().empty())
+ return false; // silently discard unknown package types
m_pkgMap.insert({pkg->name(), m_packages.size()});
m_packages.push_back(pkg);
+ return true;
}
const Package *Category::package(const string &name) const
diff --git a/src/index.hpp b/src/index.hpp
@@ -55,7 +55,7 @@ public:
Metadata *metadata() { return &m_metadata; }
const Metadata *metadata() const { return &m_metadata; }
- void addCategory(const Category *cat);
+ bool addCategory(const Category *cat);
const CategoryList &categories() const { return m_categories; }
const Category *category(size_t i) const { return m_categories[i]; }
const Category *category(const std::string &name) const;
@@ -82,7 +82,7 @@ public:
const std::string &name() const { return m_name; }
std::string fullName() const;
- void addPackage(const Package *pack);
+ bool addPackage(const Package *pack);
const PackageList &packages() const { return m_packages; }
const Package *package(size_t i) const { return m_packages[i]; }
const Package *package(const std::string &name) const;
diff --git a/src/index_v1.cpp b/src/index_v1.cpp
@@ -95,9 +95,8 @@ void LoadCategoryV1(TiXmlElement *catNode, Index *ri)
packNode = packNode->NextSiblingElement("reapack");
}
- ri->addCategory(cat);
-
- ptr.release();
+ if(ri->addCategory(cat))
+ ptr.release();
}
void LoadPackageV1(TiXmlElement *packNode, Category *cat)
@@ -129,9 +128,8 @@ void LoadPackageV1(TiXmlElement *packNode, Category *cat)
if(node)
LoadMetadataV1(node, pack->metadata());
- cat->addPackage(pack);
-
- ptr.release();
+ if(cat->addPackage(pack))
+ ptr.release();
}
void LoadVersionV1(TiXmlElement *verNode, Package *pkg)
@@ -162,9 +160,8 @@ void LoadVersionV1(TiXmlElement *verNode, Package *pkg)
ver->setChangelog(changelog);
}
- pkg->addVersion(ver);
-
- ptr.release();
+ if(pkg->addVersion(ver))
+ ptr.release();
}
void LoadSourceV1(TiXmlElement *node, Version *ver)
@@ -182,11 +179,14 @@ void LoadSourceV1(TiXmlElement *node, Version *ver)
if(!url) url = "";
Source *src = new Source(file, url, ver);
+ unique_ptr<Source> ptr(src);
+
src->setPlatform(platform);
src->setTypeOverride(Package::getType(type));
if(node->Attribute("main"))
src->setMain(true);
- ver->addSource(src);
+ if(ver->addSource(src))
+ ptr.release();
}
diff --git a/src/package.cpp b/src/package.cpp
@@ -77,15 +77,16 @@ string Package::fullName() const
return m_category ? m_category->fullName() + "/" + m_name : m_name;
}
-void Package::addVersion(const Version *ver)
+bool Package::addVersion(const Version *ver)
{
if(ver->package() != this)
throw reapack_error("version belongs to another package");
if(ver->sources().empty())
- return;
+ return false;
m_versions.insert(ver);
+ return true;
}
const Version *Package::version(const size_t index) const
diff --git a/src/package.hpp b/src/package.hpp
@@ -55,7 +55,7 @@ public:
Metadata *metadata() { return &m_metadata; }
const Metadata *metadata() const { return &m_metadata; }
- void addVersion(const Version *ver);
+ bool addVersion(const Version *ver);
const VersionSet &versions() const { return m_versions; }
const Version *version(size_t index) const;
const Version *lastVersion(bool pres = true, const Version &from = {}) const;
diff --git a/src/version.cpp b/src/version.cpp
@@ -115,12 +115,12 @@ string Version::displayAuthor() const
return m_author;
}
-void Version::addSource(Source *source)
+bool Version::addSource(const Source *source)
{
- if(!source->platform().test())
- return;
- else if(source->version() != this)
+ if(source->version() != this)
throw reapack_error("source belongs to another version");
+ else if(!source->platform().test())
+ return false;
const Path path = source->targetPath();
m_files.insert(path);
@@ -128,6 +128,8 @@ void Version::addSource(Source *source)
if(source->isMain())
m_mainSources.push_back(source);
+
+ return true;
}
void Version::setChangelog(const string &changelog)
diff --git a/src/version.hpp b/src/version.hpp
@@ -33,7 +33,7 @@ class Path;
class Version {
public:
- typedef std::vector<Source *> SourceList;
+ typedef std::vector<const Source *> SourceList;
typedef std::multimap<Path, const Source *> SourceMap;
Version();
@@ -61,7 +61,7 @@ public:
void setChangelog(const std::string &);
const std::string &changelog() const { return m_changelog; }
- void addSource(Source *source);
+ bool addSource(const Source *source);
const SourceMap &sources() const { return m_sources; }
const Source *source(size_t) const;
const SourceList &mainSources() const { return m_mainSources; }
diff --git a/test/index.cpp b/test/index.cpp
@@ -101,7 +101,7 @@ TEST_CASE("add a category", M) {
CHECK(ri.categories().size() == 0);
CHECK(ri.category("a") == nullptr);
- ri.addCategory(cat);
+ REQUIRE(ri.addCategory(cat));
REQUIRE(ri.categories().size() == 1);
REQUIRE(ri.category("a") == cat);
@@ -126,7 +126,8 @@ TEST_CASE("add owned category", M) {
TEST_CASE("drop empty category", M) {
Index ri("a");
- ri.addCategory(new Category("a", &ri));
+ const Category cat("a", &ri);
+ REQUIRE_FALSE(ri.addCategory(&cat));
REQUIRE(ri.categories().empty());
}
@@ -142,7 +143,7 @@ TEST_CASE("add a package", M) {
CHECK(cat.packages().size() == 0);
CHECK(cat.package("name") == nullptr);
- cat.addPackage(pack);
+ REQUIRE(cat.addPackage(pack));
REQUIRE(cat.packages().size() == 1);
REQUIRE(cat.package("name") == pack);
@@ -166,15 +167,15 @@ TEST_CASE("add owned package", M) {
TEST_CASE("drop empty package", M) {
Category cat("a");
- cat.addPackage(new Package(Package::ScriptType, "name", &cat));
-
+ const Package pkg(Package::ScriptType, "name", &cat);
+ REQUIRE_FALSE(cat.addPackage(&pkg));
REQUIRE(cat.packages().empty());
}
TEST_CASE("drop unknown package", M) {
Category cat("a");
- cat.addPackage(new Package(Package::UnknownType, "name", &cat));
-
+ const Package pkg(Package::UnknownType, "name", &cat);
+ REQUIRE_FALSE(cat.addPackage(&pkg));
REQUIRE(cat.packages().size() == 0);
}
diff --git a/test/package.cpp b/test/package.cpp
@@ -78,7 +78,7 @@ TEST_CASE("package versions are sorted", M) {
Version *alpha = new Version("0.1", &pack);
alpha->addSource(new Source({}, "google.com", alpha));
- pack.addVersion(final);
+ REQUIRE(pack.addVersion(final));
REQUIRE(final->package() == &pack);
CHECK(pack.versions().size() == 1);
@@ -97,7 +97,7 @@ TEST_CASE("get latest stable version", M) {
Version *alpha = new Version("2.0-alpha", &pack);
alpha->addSource(new Source({}, "google.com", alpha));
- pack.addVersion(alpha);
+ REQUIRE(pack.addVersion(alpha));
SECTION("only prereleases are available")
REQUIRE(pack.lastVersion(false) == nullptr);
@@ -153,8 +153,8 @@ TEST_CASE("pre-release updates", M) {
TEST_CASE("drop empty version", M) {
Package pack(Package::ScriptType, "a");
- pack.addVersion(new Version("1", &pack));
-
+ const Version ver("1", &pack);
+ REQUIRE_FALSE(pack.addVersion(&ver));
REQUIRE(pack.versions().empty());
REQUIRE(pack.lastVersion() == nullptr);
}
diff --git a/test/version.cpp b/test/version.cpp
@@ -230,7 +230,7 @@ TEST_CASE("add source", M) {
CHECK(ver.sources().size() == 0);
Source *src = new Source("a", "b", &ver);
- ver.addSource(src);
+ REQUIRE(ver.addSource(src));
CHECK(ver.sources().size() == 1);
CHECK(ver.mainSources().empty());
@@ -300,10 +300,9 @@ TEST_CASE("list files", M) {
TEST_CASE("drop sources for unknown platforms", M) {
MAKE_VERSION
- Source *src = new Source("a", "b", &ver);
- src->setPlatform(Platform::UnknownPlatform);
- ver.addSource(src);
-
+ Source src("a", "b", &ver);
+ src.setPlatform(Platform::UnknownPlatform);
+ REQUIRE_FALSE(ver.addSource(&src));
REQUIRE(ver.sources().size() == 0);
}
