From 7663ca75731c4313dddbcde4d85f10383644a67a Mon Sep 17 00:00:00 2001
From: Tobias Markmann <tm@ayena.de>
Date: Thu, 30 Mar 2017 09:32:28 +0200
Subject: 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

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;
-- 
cgit v0.10.2-6-g49f6