diff --git a/simgear/io/HTTPRepository.cxx b/simgear/io/HTTPRepository.cxx index b3f6e71e..a396a2bb 100644 --- a/simgear/io/HTTPRepository.cxx +++ b/simgear/io/HTTPRepository.cxx @@ -49,6 +49,7 @@ namespace simgear { class HTTPDirectory; + using HTTPDirectory_ptr = std::unique_ptr; class HTTPRepoGetRequest : public HTTP::Request { @@ -137,7 +138,7 @@ public: SGPath basePath; bool isUpdating; HTTPRepository::ResultCode status; - HTTPDirectory* rootDir; + HTTPDirectory_ptr rootDir; size_t totalDownloaded; HTTP::Request_ptr updateFile(HTTPDirectory* dir, const std::string& name, @@ -165,7 +166,7 @@ public: HTTPDirectory* getOrCreateDirectory(const std::string& path); bool deleteDirectory(const std::string& relPath, const SGPath& absPath); - typedef std::vector DirectoryVector; + typedef std::vector DirectoryVector; DirectoryVector directories; SGPath installedCopyPath; @@ -597,7 +598,7 @@ private: void removeChild(SGPath path) { bool ok; - SG_LOG(SG_TERRASYNC, SG_WARN, "Removing:" << path); + SG_LOG(SG_TERRASYNC, SG_INFO, "Removing:" << path); std::string fpath = _relativePath + "/" + path.file(); if (path.isDir()) { @@ -631,14 +632,7 @@ HTTPRepository::HTTPRepository(const SGPath& base, HTTP::Client *cl) : { _d->http = cl; _d->basePath = base; - // REVIEW: Memory Leak - 885,598 (3,072 direct, 882,526 indirect) bytes in 48 blocks are definitely lost - // _d->rootDir should be changed to a shared_ptr - // this object is passed to several chained methods, so destruction within ~HTTPRepoPrivate may be - // an easier fix assuming sole ownership would apply - // REVIEW: _d is only partially initialized when it is being passed to the HTTPDirectory ctor. - // The HTTPDirectory object is taking ownership of the shared HTTPRepoPrivate object. - // Mismatch between _d being a unique_ptr, yet also cloned into HTTPDirectory. - _d->rootDir = new HTTPDirectory(_d.get(), ""); + _d->rootDir.reset(new HTTPDirectory(_d.get(), "")); _d->parseHashCache(); } @@ -675,7 +669,7 @@ void HTTPRepository::update() _d->status = REPO_NO_ERROR; _d->isUpdating = true; _d->failures.clear(); - _d->updateDir(_d->rootDir, std::string(), 0); + _d->updateDir(_d->rootDir.get(), {}, 0); } bool HTTPRepository::isDoingSync() const @@ -943,10 +937,7 @@ HTTPRepository::failure() const http->cancelRequest(*rq, "Repository object deleted"); } - DirectoryVector::iterator it; - for (it=directories.begin(); it != directories.end(); ++it) { - delete *it; - } + directories.clear(); // wil delete them all } HTTP::Request_ptr HTTPRepoPrivate::updateFile(HTTPDirectory* dir, const std::string& name, size_t sz) @@ -1109,7 +1100,7 @@ HTTPRepository::failure() const { public: DirectoryWithPath(const std::string& p) : path(p) {} - bool operator()(const HTTPDirectory* entry) const + bool operator()(const HTTPDirectory_ptr& entry) const { return entry->relativePath() == path; } private: std::string path; @@ -1118,14 +1109,13 @@ HTTPRepository::failure() const HTTPDirectory* HTTPRepoPrivate::getOrCreateDirectory(const std::string& path) { DirectoryWithPath p(path); - DirectoryVector::iterator it = std::find_if(directories.begin(), directories.end(), p); + auto it = std::find_if(directories.begin(), directories.end(), p); if (it != directories.end()) { - return *it; + return it->get(); } - HTTPDirectory* d = new HTTPDirectory(this, path); - directories.push_back(d); - return d; + directories.emplace_back(new HTTPDirectory(this, path)); + return directories.back().get(); } bool HTTPRepoPrivate::deleteDirectory(const std::string& relPath, const SGPath& absPath) @@ -1133,10 +1123,9 @@ HTTPRepository::failure() const DirectoryWithPath p(relPath); auto it = std::find_if(directories.begin(), directories.end(), p); if (it != directories.end()) { - HTTPDirectory* d = *it; + HTTPDirectory* d = it->get(); assert(d->absolutePath() == absPath); directories.erase(it); - delete d; } else { // we encounter this code path when deleting an orphaned directory }