diff options
author | Tobias Markmann <tm@ayena.de> | 2017-03-30 07:32:28 (GMT) |
---|---|---|
committer | Tobias Markmann <tm@ayena.de> | 2017-03-31 08:45:43 (GMT) |
commit | 7663ca75731c4313dddbcde4d85f10383644a67a (patch) | |
tree | ecb1ed4c08b71bb58efc61980166e5064fefe68e | |
parent | 4d0391d824aaf94fbe152778581d51fecd588f6c (diff) | |
download | swift-7663ca75731c4313dddbcde4d85f10383644a67a.zip swift-7663ca75731c4313dddbcde4d85f10383644a67a.tar.bz2 |
Return unique_ptr instead of pointer to deleted object
Coverity raised this issue.
Test-Information:
Code builds on macOS 10.12.4 and all unit tests pass; Swift
runs fine.
Change-Id: I8fb0805f6b2e0a21674ea32c0b1aee9e7b985639
-rw-r--r-- | Swift/Controllers/Roster/GroupRosterItem.cpp | 27 | ||||
-rw-r--r-- | Swift/Controllers/Roster/GroupRosterItem.h | 20 | ||||
-rw-r--r-- | Swift/Controllers/Roster/Roster.cpp | 4 |
3 files changed, 29 insertions, 22 deletions
diff --git a/Swift/Controllers/Roster/GroupRosterItem.cpp b/Swift/Controllers/Roster/GroupRosterItem.cpp index ac40afd..af5d0ca 100644 --- a/Swift/Controllers/Roster/GroupRosterItem.cpp +++ b/Swift/Controllers/Roster/GroupRosterItem.cpp @@ -1,13 +1,14 @@ /* - * Copyright (c) 2010-2016 Isode Limited. + * Copyright (c) 2010-2017 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ #include <Swift/Controllers/Roster/GroupRosterItem.h> + +#include <memory> + #include <boost/bind.hpp> -//#include <boost/algorithm.hpp> -#include <iostream> namespace Swift { @@ -97,23 +98,22 @@ void GroupRosterItem::removeAll() { * Returns the removed item - but only if it's the only one, otherwise * the return result is undefined. */ -ContactRosterItem* GroupRosterItem::removeChild(const JID& jid) { - std::vector<RosterItem*>::iterator it = children_.begin(); - ContactRosterItem* removed = nullptr; +std::unique_ptr<ContactRosterItem> GroupRosterItem::removeChild(const JID& jid) { + auto it = children_.begin(); + std::unique_ptr<ContactRosterItem> removed; while (it != children_.end()) { ContactRosterItem* contact = dynamic_cast<ContactRosterItem*>(*it); if (contact && contact->getJID() == jid) { displayedChildren_.erase(std::remove(displayedChildren_.begin(), displayedChildren_.end(), contact), displayedChildren_.end()); - removed = contact; - delete contact; + removed = std::unique_ptr<ContactRosterItem>(contact); it = children_.erase(it); continue; } GroupRosterItem* group = dynamic_cast<GroupRosterItem*>(*it); if (group) { - ContactRosterItem* groupRemoved = group->removeChild(jid); + auto groupRemoved = group->removeChild(jid); if (groupRemoved) { - removed = groupRemoved; + removed = std::move(groupRemoved); } } ++it; @@ -123,15 +123,14 @@ ContactRosterItem* GroupRosterItem::removeChild(const JID& jid) { return removed; } -GroupRosterItem* GroupRosterItem::removeGroupChild(const std::string& groupName) { +std::unique_ptr<GroupRosterItem> GroupRosterItem::removeGroupChild(const std::string& groupName) { std::vector<RosterItem*>::iterator it = children_.begin(); - GroupRosterItem* removed = nullptr; + std::unique_ptr<GroupRosterItem> removed; while (it != children_.end()) { GroupRosterItem* group = dynamic_cast<GroupRosterItem*>(*it); if (group && group->getDisplayName() == groupName) { displayedChildren_.erase(std::remove(displayedChildren_.begin(), displayedChildren_.end(), group), displayedChildren_.end()); - removed = group; - delete group; + removed = std::unique_ptr<GroupRosterItem>(group); it = children_.erase(it); continue; } diff --git a/Swift/Controllers/Roster/GroupRosterItem.h b/Swift/Controllers/Roster/GroupRosterItem.h index a4e008f..ebb3849 100644 --- a/Swift/Controllers/Roster/GroupRosterItem.h +++ b/Swift/Controllers/Roster/GroupRosterItem.h @@ -18,25 +18,33 @@ class GroupRosterItem : public RosterItem { public: GroupRosterItem(const std::string& name, GroupRosterItem* parent, bool sortByStatus); virtual ~GroupRosterItem(); + const std::vector<RosterItem*>& getChildren() const; const std::vector<RosterItem*>& getDisplayedChildren() const; + void addChild(RosterItem* item); - ContactRosterItem* removeChild(const JID& jid); - GroupRosterItem* removeGroupChild(const std::string& group); + std::unique_ptr<ContactRosterItem> removeChild(const JID& jid); + std::unique_ptr<GroupRosterItem> removeGroupChild(const std::string& group); void removeAll(); + void setDisplayed(RosterItem* item, bool displayed); - boost::signals2::signal<void ()> onChildrenChanged; - static bool itemLessThanWithStatus(const RosterItem* left, const RosterItem* right); - static bool itemLessThanWithoutStatus(const RosterItem* left, const RosterItem* right); void setExpanded(bool expanded); bool isExpanded() const; - boost::signals2::signal<void (bool)> onExpandedChanged; void setManualSort(const std::string& manualSortValue); virtual const std::string& getSortableDisplayName() const; + + boost::signals2::signal<void (bool)> onExpandedChanged; + boost::signals2::signal<void ()> onChildrenChanged; + + static bool itemLessThanWithStatus(const RosterItem* left, const RosterItem* right); + static bool itemLessThanWithoutStatus(const RosterItem* left, const RosterItem* right); + private: void handleChildrenChanged(GroupRosterItem* group); void handleDataChanged(RosterItem* item); bool sortDisplayed(); + + private: std::string name_; bool expanded_; std::vector<RosterItem*> children_; diff --git a/Swift/Controllers/Roster/Roster.cpp b/Swift/Controllers/Roster/Roster.cpp index 9c9b9e3..f6f6ce0 100644 --- a/Swift/Controllers/Roster/Roster.cpp +++ b/Swift/Controllers/Roster/Roster.cpp @@ -164,10 +164,10 @@ void Roster::removeContactFromGroup(const JID& jid, const std::string& groupName while (it != children.end()) { GroupRosterItem* group = dynamic_cast<GroupRosterItem*>(*it); if (group && group->getDisplayName() == groupName) { - ContactRosterItem* deleted = group->removeChild(jid); + auto deleted = group->removeChild(jid); if (itemIt != itemMap_.end()) { std::vector<ContactRosterItem*>& items = itemIt->second; - items.erase(std::remove(items.begin(), items.end(), deleted), items.end()); + items.erase(std::remove(items.begin(), items.end(), deleted.get()), items.end()); } } ++it; |