From a545481daa4c4a6173e48f6aaee5e6376d75c617 Mon Sep 17 00:00:00 2001
From: csoler <csoler@users.sourceforge.net>
Date: Thu, 9 Mar 2017 22:05:06 +0100
Subject: [PATCH] fixed memory leak when receving multi-chunk file lists

---
 libretroshare/src/file_sharing/p3filelists.cc | 23 ++++++++++++++++---
 libretroshare/src/file_sharing/p3filelists.h  |  2 +-
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/libretroshare/src/file_sharing/p3filelists.cc b/libretroshare/src/file_sharing/p3filelists.cc
index 35725e412..2a35dfdc5 100644
--- a/libretroshare/src/file_sharing/p3filelists.cc
+++ b/libretroshare/src/file_sharing/p3filelists.cc
@@ -1189,8 +1189,12 @@ void p3FileDatabase::tickRecv()
       {
       case RS_PKT_SUBTYPE_FILELISTS_SYNC_REQ_ITEM: handleDirSyncRequest( dynamic_cast<RsFileListsSyncRequestItem*>(item) ) ;
          break ;
-      case RS_PKT_SUBTYPE_FILELISTS_SYNC_RSP_ITEM: handleDirSyncResponse( dynamic_cast<RsFileListsSyncResponseItem*>(item) ) ;
-         break ;
+      case RS_PKT_SUBTYPE_FILELISTS_SYNC_RSP_ITEM:
+	  {
+          RsFileListsSyncResponseItem *sitem = dynamic_cast<RsFileListsSyncResponseItem*>(item);
+		  handleDirSyncResponse(sitem) ;
+      }
+		  break ;
       default:
          P3FILELISTS_ERROR() << "(EE) unhandled packet subtype " << item->PacketSubType() << " in " << __PRETTY_FUNCTION__ << std::endl;
       }
@@ -1322,6 +1326,7 @@ void p3FileDatabase::splitAndSendItem(RsFileListsSyncResponseItem *ritem)
 }
 
 // This function should not take memory ownership of ritem, so it makes copies.
+// The item that is returned is either created (if different from ritem) or equal to ritem.
 
 RsFileListsSyncResponseItem *p3FileDatabase::recvAndRebuildItem(RsFileListsSyncResponseItem *ritem)
 {
@@ -1393,13 +1398,25 @@ RsFileListsSyncResponseItem *p3FileDatabase::recvAndRebuildItem(RsFileListsSyncR
         return NULL ;
 }
 
-void p3FileDatabase::handleDirSyncResponse(RsFileListsSyncResponseItem *sitem)
+// We employ a trick in this function:
+// - if recvAndRebuildItem(item) returns the same item, it has not created memory, so the incoming item should be the one to
+//   delete, which is done by the caller in every case.
+// - if it returns a different item, it means that the item has been created below when collapsing items, so we should delete both.
+//   to do so, we first delete the incoming item, and replace the pointer by the new created one.
+
+void p3FileDatabase::handleDirSyncResponse(RsFileListsSyncResponseItem*& sitem)
 {
     RsFileListsSyncResponseItem *item = recvAndRebuildItem(sitem) ;
 
     if(!item)
         return ;
 
+    if(item != sitem)
+    {
+        delete sitem ;
+        sitem = item ;
+    }
+
     time_t now = time(NULL);
 
     // check the hash. If anything goes wrong (in the chunking for instance) the hash will not match
diff --git a/libretroshare/src/file_sharing/p3filelists.h b/libretroshare/src/file_sharing/p3filelists.h
index 830986f2b..81630bb74 100644
--- a/libretroshare/src/file_sharing/p3filelists.h
+++ b/libretroshare/src/file_sharing/p3filelists.h
@@ -205,7 +205,7 @@ class p3FileDatabase: public p3Service, public p3Config, public ftSearch //, pub
         uint32_t locked_getFriendIndex(const RsPeerId& pid);
 
         void handleDirSyncRequest (RsFileListsSyncRequestItem *) ;
-        void handleDirSyncResponse (RsFileListsSyncResponseItem *) ;
+        void handleDirSyncResponse (RsFileListsSyncResponseItem *&) ;
 
         std::map<RsPeerId,uint32_t> mFriendIndexMap ;
         std::vector<RsPeerId> mFriendIndexTab;